You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@seata.apache.org by "Xb2555 (via GitHub)" <gi...@apache.org> on 2025/12/19 09:56:43 UTC
[PR] feature: add MCP custom configuration and authentication code [incubator-seata]
Xb2555 opened a new pull request, #7876:
URL: https://github.com/apache/incubator-seata/pull/7876
<!--
Licensed to the Apache Software Foundation (ASF) under one or more
contributor license agreements. See the NOTICE file distributed with
this work for additional information regarding copyright ownership.
The ASF licenses this file to You under the Apache License, Version 2.0
(the "License"); you may not use this file except in compliance with
the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->
<!-- Please make sure you have read and understood the contributing guidelines -->
- [x] I have read the [CONTRIBUTING.md](https://github.com/apache/incubator-seata/blob/2.x/CONTRIBUTING.md) guidelines.
- [x] I have registered the PR [changes](https://github.com/apache/incubator-seata/tree/2.x/changes).
### Ⅰ. Describe what this PR did
1. Add configuration file attributes related to MCP
3. Implement authentication for request paths related to MCP
### Ⅱ. Does this pull request fix one issue?
<!-- If that, add "fixes #xxx" below in the next line, for example, fixes #97. -->
### Ⅲ. Why don't you add test cases (unit test/integration test)?
### Ⅳ. Describe how to verify it
### Ⅴ. Special notes for reviews
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: add MCP custom configuration and authentication code [incubator-seata]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #7876:
URL: https://github.com/apache/incubator-seata/pull/7876#discussion_r2636963475
##########
console/src/main/java/org/apache/seata/mcp/core/props/MCPProperties.java:
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.mcp.core.props;
+
+import jakarta.annotation.PostConstruct;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.core.env.Environment;
+import org.springframework.stereotype.Component;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+@Component
+public class MCPProperties {
+
+ public static final String SSE_TYPE = "sse";
+ public static final String STREAMABLE_TYPE = "streamable";
+
+ private boolean enableAuth = true;
+
+ private Long queryDuration = TimeUnit.DAYS.toMillis(1);
+
+ private String mcpType = SSE_TYPE;
+
+ private StreamableProperties streamableProperties;
+
+ private SseServerProperties sseServerProperties;
+
+ public MCPProperties() {}
+
+ public boolean isSseType() {
+ return mcpType.equals(SSE_TYPE);
+ }
+
+ public List<String> getEndpoints() {
+ List<String> result = new ArrayList<>();
+ if (isSseType()) {
+ result.add(sseServerProperties.sseEndpoint);
+ result.add(sseServerProperties.messageEndpoint);
+ } else {
+ result.add(streamableProperties.mcpEndPoint);
+ }
+ return result;
Review Comment:
The getEndpoints method can throw a NullPointerException if called before the init method completes, or if init fails to initialize the properties. When mcpType is SSE_TYPE, sseServerProperties might be null. When mcpType is not SSE_TYPE, streamableProperties might be null. Consider adding null checks or ensuring proper initialization validation.
##########
console/src/main/java/org/apache/seata/console/filter/MCPAuthenticationFilter.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.console.filter;
+
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import org.apache.seata.console.security.User;
+import org.springframework.http.HttpStatus;
+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.web.filter.OncePerRequestFilter;
+
+import java.io.IOException;
+
+public class MCPAuthenticationFilter extends OncePerRequestFilter {
+
+ private final AuthenticationManager authenticationManager;
+
+ /**
+ * Instantiates a new Jwt authentication token filter.
+ *
+ * @param authenticationManager the token provider
+ */
+ public MCPAuthenticationFilter(AuthenticationManager authenticationManager) {
+ this.authenticationManager = authenticationManager;
+ }
+
+ @Override
+ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain chain)
+ throws IOException, ServletException {
+
+ Authentication existingAuth = SecurityContextHolder.getContext().getAuthentication();
+ if (existingAuth != null && existingAuth.isAuthenticated()) {
+ chain.doFilter(request, response);
+ return;
+ }
+
+ User user = resolveHeaders(request);
+
+ UsernamePasswordAuthenticationToken authenticationToken =
+ new UsernamePasswordAuthenticationToken(user.getUsername(), user.getPassword());
+
+ try {
+ Authentication authentication = authenticationManager.authenticate(authenticationToken);
+ SecurityContextHolder.getContext().setAuthentication(authentication);
+ } catch (BadCredentialsException e) {
+ response.sendError(HttpStatus.UNAUTHORIZED.value(), "Invalid credentials");
+ return;
+ }
+
+ chain.doFilter(request, response);
+ }
+
+ /**
+ * Get userDetails from header
+ */
+ private User resolveHeaders(HttpServletRequest request) {
+ String username = request.getHeader("X-Mcp-Username");
+ String password = request.getHeader("X-Mcp-Password");
+ return new User(username, password);
+ }
Review Comment:
Sensitive credentials (username and password) are being transmitted in custom HTTP headers. This approach can lead to credentials being logged by proxies, load balancers, or application logs. Consider using standard authentication mechanisms like HTTP Basic Authentication or Bearer tokens, which have better tooling support and are less likely to be inadvertently logged.
##########
console/src/main/java/org/apache/seata/console/filter/MCPAuthenticationFilter.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.console.filter;
+
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import org.apache.seata.console.security.User;
+import org.springframework.http.HttpStatus;
+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.web.filter.OncePerRequestFilter;
+
+import java.io.IOException;
+
+public class MCPAuthenticationFilter extends OncePerRequestFilter {
+
+ private final AuthenticationManager authenticationManager;
+
+ /**
+ * Instantiates a new Jwt authentication token filter.
+ *
+ * @param authenticationManager the token provider
Review Comment:
The Javadoc comment incorrectly states "Instantiates a new Jwt authentication token filter" and refers to "the token provider". This should be updated to reflect that this is an MCP authentication filter that uses username/password from headers, not JWT tokens.
```suggestion
* Instantiates a new MCP authentication filter that authenticates requests
* using username and password provided in HTTP headers.
*
* @param authenticationManager the authentication manager used to validate credentials
```
##########
console/src/main/java/org/apache/seata/console/config/MCPFiltersConfig.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.console.config;
+
+import org.apache.seata.console.filter.MCPAuthenticationFilter;
+import org.apache.seata.mcp.core.props.MCPProperties;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.web.servlet.FilterRegistrationBean;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.core.Ordered;
+import org.springframework.security.authentication.AuthenticationManager;
+
+@Configuration
+public class MCPFiltersConfig {
+
+ @Autowired
+ private AuthenticationManager authenticationManager;
+
+ @Autowired
+ private MCPProperties mcpProperties;
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(MCPFiltersConfig.class);
+
+ @Bean
+ public FilterRegistrationBean<MCPAuthenticationFilter> mcpJwtAuthenticationTokenFilterRegistration() {
+
+ MCPAuthenticationFilter mcpJwtAuthenticationTokenFilter = new MCPAuthenticationFilter(authenticationManager);
+
+ FilterRegistrationBean<MCPAuthenticationFilter> registration = new FilterRegistrationBean<>();
+
+ registration.setFilter(mcpJwtAuthenticationTokenFilter);
+
+ for (String endPoint : mcpProperties.getEndpoints()) {
+ registration.addUrlPatterns(endPoint);
+ }
+
+ registration.setName("mcpJwtAuthenticationTokenFilter");
Review Comment:
The method name 'mcpJwtAuthenticationTokenFilterRegistration' and variable 'mcpJwtAuthenticationTokenFilter' reference 'Jwt' but this filter performs basic username/password authentication, not JWT authentication. The naming should reflect the actual authentication mechanism, such as 'mcpAuthenticationFilterRegistration' and 'mcpAuthenticationFilter'.
```suggestion
public FilterRegistrationBean<MCPAuthenticationFilter> mcpAuthenticationFilterRegistration() {
MCPAuthenticationFilter mcpAuthenticationFilter = new MCPAuthenticationFilter(authenticationManager);
FilterRegistrationBean<MCPAuthenticationFilter> registration = new FilterRegistrationBean<>();
registration.setFilter(mcpAuthenticationFilter);
for (String endPoint : mcpProperties.getEndpoints()) {
registration.addUrlPatterns(endPoint);
}
registration.setName("mcpAuthenticationFilter");
```
##########
namingserver/src/main/resources/application.yml:
##########
@@ -40,3 +48,10 @@ seata:
csrf-ignore-urls: /naming/v1/**,/api/v1/naming/**
ignore:
urls: /,/**/*.css,/**/*.js,/**/*.html,/**/*.map,/**/*.svg,/**/*.png,/**/*.jpeg,/**/*.ico,/api/v1/auth/login,/version.json,/naming/v1/health,/error
+ # MCP Server Configuration
+ mcp:
+ # Query limits the maximum time interval, The unit is milliseconds, Default one day
+ query:
+ max_query_duration: 86400000
Review Comment:
The configuration property uses snake_case 'max_query_duration' which is inconsistent with typical Spring Boot property naming conventions that use kebab-case. This should be 'max-query-duration' to follow Spring Boot conventions.
```suggestion
max-query-duration: 86400000
```
##########
console/src/main/java/org/apache/seata/console/filter/MCPAuthenticationFilter.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.console.filter;
+
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import org.apache.seata.console.security.User;
+import org.springframework.http.HttpStatus;
+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.web.filter.OncePerRequestFilter;
+
+import java.io.IOException;
+
+public class MCPAuthenticationFilter extends OncePerRequestFilter {
+
+ private final AuthenticationManager authenticationManager;
+
+ /**
+ * Instantiates a new Jwt authentication token filter.
+ *
+ * @param authenticationManager the token provider
+ */
+ public MCPAuthenticationFilter(AuthenticationManager authenticationManager) {
+ this.authenticationManager = authenticationManager;
+ }
+
+ @Override
+ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain chain)
+ throws IOException, ServletException {
+
+ Authentication existingAuth = SecurityContextHolder.getContext().getAuthentication();
+ if (existingAuth != null && existingAuth.isAuthenticated()) {
+ chain.doFilter(request, response);
+ return;
+ }
+
+ User user = resolveHeaders(request);
+
+ UsernamePasswordAuthenticationToken authenticationToken =
+ new UsernamePasswordAuthenticationToken(user.getUsername(), user.getPassword());
+
+ try {
+ Authentication authentication = authenticationManager.authenticate(authenticationToken);
+ SecurityContextHolder.getContext().setAuthentication(authentication);
+ } catch (BadCredentialsException e) {
+ response.sendError(HttpStatus.UNAUTHORIZED.value(), "Invalid credentials");
+ return;
+ }
+
+ chain.doFilter(request, response);
+ }
+
+ /**
+ * Get userDetails from header
+ */
+ private User resolveHeaders(HttpServletRequest request) {
+ String username = request.getHeader("X-Mcp-Username");
+ String password = request.getHeader("X-Mcp-Password");
Review Comment:
The resolveHeaders method does not validate that username and password headers are present. If these headers are missing, null values will be passed to the User constructor and subsequently to UsernamePasswordAuthenticationToken. This could lead to unexpected behavior or NullPointerException during authentication. Consider adding validation to return a clear error when required headers are missing.
```suggestion
String password = request.getHeader("X-Mcp-Password");
if (username == null || password == null) {
throw new BadCredentialsException("Missing authentication headers");
}
```
##########
console/src/main/java/org/apache/seata/mcp/core/props/MCPProperties.java:
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.mcp.core.props;
+
+import jakarta.annotation.PostConstruct;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.core.env.Environment;
+import org.springframework.stereotype.Component;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+@Component
+public class MCPProperties {
+
+ public static final String SSE_TYPE = "sse";
+ public static final String STREAMABLE_TYPE = "streamable";
+
+ private boolean enableAuth = true;
+
+ private Long queryDuration = TimeUnit.DAYS.toMillis(1);
+
+ private String mcpType = SSE_TYPE;
+
+ private StreamableProperties streamableProperties;
+
+ private SseServerProperties sseServerProperties;
+
+ public MCPProperties() {}
+
+ public boolean isSseType() {
+ return mcpType.equals(SSE_TYPE);
+ }
+
+ public List<String> getEndpoints() {
+ List<String> result = new ArrayList<>();
+ if (isSseType()) {
+ result.add(sseServerProperties.sseEndpoint);
+ result.add(sseServerProperties.messageEndpoint);
+ } else {
+ result.add(streamableProperties.mcpEndPoint);
+ }
+ return result;
+ }
+
+ public Long getQueryDuration() {
+ return queryDuration;
+ }
+
+ public static class StreamableProperties {
+ private final String mcpEndPoint;
+
+ public StreamableProperties(String mcpEndPoint) {
+ this.mcpEndPoint = mcpEndPoint;
+ }
+ }
+
+ public static class SseServerProperties {
+
+ private final String sseEndpoint;
+
+ private final String messageEndpoint;
+
+ public SseServerProperties(String sseEndpoint, String messageEndpoint) {
+ this.sseEndpoint = sseEndpoint;
+ this.messageEndpoint = messageEndpoint;
+ }
+ }
+
+ @Autowired
+ private Environment env;
Review Comment:
Field injection is used for the Environment dependency. Constructor injection is preferred over field injection as it makes dependencies explicit, enables immutability, facilitates testing, and prevents NullPointerException risks. Consider using constructor injection instead.
##########
console/src/main/java/org/apache/seata/console/filter/MCPAuthenticationFilter.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.console.filter;
+
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import org.apache.seata.console.security.User;
+import org.springframework.http.HttpStatus;
+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.web.filter.OncePerRequestFilter;
+
+import java.io.IOException;
+
+public class MCPAuthenticationFilter extends OncePerRequestFilter {
+
+ private final AuthenticationManager authenticationManager;
+
+ /**
+ * Instantiates a new Jwt authentication token filter.
+ *
+ * @param authenticationManager the token provider
+ */
+ public MCPAuthenticationFilter(AuthenticationManager authenticationManager) {
+ this.authenticationManager = authenticationManager;
+ }
+
+ @Override
+ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain chain)
+ throws IOException, ServletException {
+
+ Authentication existingAuth = SecurityContextHolder.getContext().getAuthentication();
+ if (existingAuth != null && existingAuth.isAuthenticated()) {
+ chain.doFilter(request, response);
+ return;
+ }
Review Comment:
The authentication check at line 52 uses a weak condition. An authentication object can exist but represent an anonymous user. The check should verify both that authentication exists and that it represents a properly authenticated, non-anonymous user. Consider using `existingAuth.isAuthenticated() && !(existingAuth instanceof AnonymousAuthenticationToken)`.
##########
namingserver/src/main/resources/application.yml:
##########
@@ -40,3 +48,10 @@ seata:
csrf-ignore-urls: /naming/v1/**,/api/v1/naming/**
ignore:
urls: /,/**/*.css,/**/*.js,/**/*.html,/**/*.map,/**/*.svg,/**/*.png,/**/*.jpeg,/**/*.ico,/api/v1/auth/login,/version.json,/naming/v1/health,/error
+ # MCP Server Configuration
+ mcp:
+ # Query limits the maximum time interval, The unit is milliseconds, Default one day
Review Comment:
Comment says "Query limits the maximum time interval" but should say "Query limit for the maximum time interval" or "Maximum query time interval". The plural form "limits" is grammatically incorrect here.
```suggestion
# Maximum query time interval, The unit is milliseconds, Default one day
```
##########
console/src/main/java/org/apache/seata/console/filter/MCPAuthenticationFilter.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.console.filter;
+
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import org.apache.seata.console.security.User;
+import org.springframework.http.HttpStatus;
+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.web.filter.OncePerRequestFilter;
+
+import java.io.IOException;
+
+public class MCPAuthenticationFilter extends OncePerRequestFilter {
+
+ private final AuthenticationManager authenticationManager;
+
+ /**
+ * Instantiates a new Jwt authentication token filter.
+ *
+ * @param authenticationManager the token provider
+ */
+ public MCPAuthenticationFilter(AuthenticationManager authenticationManager) {
+ this.authenticationManager = authenticationManager;
+ }
+
+ @Override
+ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain chain)
+ throws IOException, ServletException {
+
+ Authentication existingAuth = SecurityContextHolder.getContext().getAuthentication();
+ if (existingAuth != null && existingAuth.isAuthenticated()) {
+ chain.doFilter(request, response);
+ return;
+ }
+
+ User user = resolveHeaders(request);
+
+ UsernamePasswordAuthenticationToken authenticationToken =
+ new UsernamePasswordAuthenticationToken(user.getUsername(), user.getPassword());
+
+ try {
+ Authentication authentication = authenticationManager.authenticate(authenticationToken);
+ SecurityContextHolder.getContext().setAuthentication(authentication);
+ } catch (BadCredentialsException e) {
+ response.sendError(HttpStatus.UNAUTHORIZED.value(), "Invalid credentials");
+ return;
+ }
Review Comment:
The code only catches BadCredentialsException but the authenticationManager.authenticate() can throw other AuthenticationException subtypes such as DisabledException, LockedException, or AccountExpiredException. These exceptions would propagate uncaught, potentially exposing stack traces. Consider catching the broader AuthenticationException to handle all authentication failures gracefully.
##########
console/src/main/java/org/apache/seata/mcp/core/props/MCPProperties.java:
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.mcp.core.props;
+
+import jakarta.annotation.PostConstruct;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.core.env.Environment;
+import org.springframework.stereotype.Component;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+@Component
+public class MCPProperties {
+
+ public static final String SSE_TYPE = "sse";
+ public static final String STREAMABLE_TYPE = "streamable";
+
+ private boolean enableAuth = true;
+
+ private Long queryDuration = TimeUnit.DAYS.toMillis(1);
+
+ private String mcpType = SSE_TYPE;
+
+ private StreamableProperties streamableProperties;
+
+ private SseServerProperties sseServerProperties;
+
+ public MCPProperties() {}
+
+ public boolean isSseType() {
+ return mcpType.equals(SSE_TYPE);
+ }
+
+ public List<String> getEndpoints() {
+ List<String> result = new ArrayList<>();
+ if (isSseType()) {
+ result.add(sseServerProperties.sseEndpoint);
+ result.add(sseServerProperties.messageEndpoint);
+ } else {
+ result.add(streamableProperties.mcpEndPoint);
+ }
+ return result;
+ }
+
+ public Long getQueryDuration() {
+ return queryDuration;
+ }
+
+ public static class StreamableProperties {
+ private final String mcpEndPoint;
+
+ public StreamableProperties(String mcpEndPoint) {
+ this.mcpEndPoint = mcpEndPoint;
+ }
+ }
+
+ public static class SseServerProperties {
+
+ private final String sseEndpoint;
+
+ private final String messageEndpoint;
+
+ public SseServerProperties(String sseEndpoint, String messageEndpoint) {
+ this.sseEndpoint = sseEndpoint;
+ this.messageEndpoint = messageEndpoint;
+ }
+ }
+
+ @Autowired
+ private Environment env;
+
+ @PostConstruct
+ public void init() {
+ mcpType = env.getProperty("spring.ai.mcp.server.protocol", "sse");
+ if (mcpType.equals(STREAMABLE_TYPE)) {
+ String mcpEndPoint = env.getProperty("spring.ai.mcp.server.streamable-http.mcp-endpoint", "/mcp");
+ streamableProperties = new StreamableProperties(mcpEndPoint);
+ } else {
+ mcpType = SSE_TYPE;
+ String sseEndpoint = env.getProperty("spring.ai.mcp.server.sse-endpoint", "/sse");
+ String messageEndpoint = env.getProperty("spring.ai.mcp.server.sse-message-endpoint", "/mcp/message");
+ sseServerProperties = new SseServerProperties(sseEndpoint, messageEndpoint);
+ }
+ queryDuration = Long.parseLong(env.getProperty("seata.mcp.query.max_query_duration", "86400000"));
Review Comment:
Potential uncaught 'java.lang.NumberFormatException'.
```suggestion
String maxQueryDurationStr = env.getProperty("seata.mcp.query.max_query_duration", "86400000");
try {
queryDuration = Long.parseLong(maxQueryDurationStr);
} catch (NumberFormatException ex) {
queryDuration = TimeUnit.DAYS.toMillis(1);
}
```
##########
console/src/main/java/org/apache/seata/mcp/core/props/MCPProperties.java:
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.mcp.core.props;
+
+import jakarta.annotation.PostConstruct;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.core.env.Environment;
+import org.springframework.stereotype.Component;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+@Component
+public class MCPProperties {
+
+ public static final String SSE_TYPE = "sse";
+ public static final String STREAMABLE_TYPE = "streamable";
+
+ private boolean enableAuth = true;
+
+ private Long queryDuration = TimeUnit.DAYS.toMillis(1);
+
+ private String mcpType = SSE_TYPE;
+
+ private StreamableProperties streamableProperties;
+
+ private SseServerProperties sseServerProperties;
+
+ public MCPProperties() {}
+
+ public boolean isSseType() {
+ return mcpType.equals(SSE_TYPE);
+ }
+
+ public List<String> getEndpoints() {
+ List<String> result = new ArrayList<>();
+ if (isSseType()) {
+ result.add(sseServerProperties.sseEndpoint);
+ result.add(sseServerProperties.messageEndpoint);
+ } else {
+ result.add(streamableProperties.mcpEndPoint);
+ }
+ return result;
+ }
+
+ public Long getQueryDuration() {
+ return queryDuration;
+ }
+
+ public static class StreamableProperties {
+ private final String mcpEndPoint;
Review Comment:
The naming convention 'mcpEndPoint' uses inconsistent capitalization - 'Point' should be 'point' (camelCase). For consistency with Java naming conventions, this should be 'mcpEndpoint'.
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: add MCP custom configuration and authentication code [incubator-seata]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #7876:
URL: https://github.com/apache/incubator-seata/pull/7876#discussion_r2641829931
##########
console/src/main/java/org/apache/seata/console/filter/MCPAuthenticationFilter.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.console.filter;
+
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import org.apache.seata.console.security.User;
+import org.springframework.http.HttpStatus;
+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.AuthenticationException;
+import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.web.filter.OncePerRequestFilter;
+
+import java.io.IOException;
+
+public class MCPAuthenticationFilter extends OncePerRequestFilter {
+
+ private final AuthenticationManager authenticationManager;
+
+ /**
+ * Instantiates a new MCP authentication filter that authenticates requests
+ * using username and password provided in HTTP headers.
+ *
+ * @param authenticationManager the authentication manager used to validate credentials
+ */
+ public MCPAuthenticationFilter(AuthenticationManager authenticationManager) {
+ this.authenticationManager = authenticationManager;
+ }
+
+ @Override
+ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain chain)
+ throws IOException, ServletException {
+
+ Authentication existingAuth = SecurityContextHolder.getContext().getAuthentication();
+ if (existingAuth != null && existingAuth.isAuthenticated()) {
+ chain.doFilter(request, response);
+ return;
+ }
+
+ User user = resolveHeaders(request);
+
+ UsernamePasswordAuthenticationToken authenticationToken =
+ new UsernamePasswordAuthenticationToken(user.getUsername(), user.getPassword());
Review Comment:
The User object is being instantiated with a plaintext password from the request header. This password should not be stored or passed around in plaintext. The authentication process should use the password directly with the AuthenticationManager without creating an intermediate User object that holds plaintext credentials. Consider passing the credentials directly to the UsernamePasswordAuthenticationToken without creating a User object.
##########
console/src/main/java/org/apache/seata/mcp/core/props/MCPProperties.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.mcp.core.props;
+
+import jakarta.annotation.PostConstruct;
+import org.springframework.core.env.Environment;
+import org.springframework.stereotype.Component;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+@Component
+public class MCPProperties {
+
+ private final Environment env;
+
+ public static final String SSE_TYPE = "sse";
+
+ public static final String STREAMABLE_TYPE = "streamable";
+
+ private boolean enableAuth = true;
+
+ private Long queryDuration = TimeUnit.DAYS.toMillis(1);
+
+ private String mcpType = SSE_TYPE;
+
+ private StreamableProperties streamableProperties;
+
+ private SseServerProperties sseServerProperties;
+
+ public MCPProperties(Environment env) {
+ this.env = env;
+ }
+
+ public boolean isSseType() {
+ return mcpType.equals(SSE_TYPE);
+ }
+
+ public List<String> getEndpoints() {
+ List<String> result = new ArrayList<>();
+ if (isSseType()) {
+ result.add(sseServerProperties.sseEndpoint);
+ result.add(sseServerProperties.messageEndpoint);
+ } else {
+ result.add(streamableProperties.mcpEndpoint);
+ }
+ return result;
+ }
+
+ public Long getQueryDuration() {
+ return queryDuration;
+ }
+
+ public static class StreamableProperties {
+ private final String mcpEndpoint;
+
+ public StreamableProperties(String mcpEndPoint) {
+ this.mcpEndpoint = mcpEndPoint;
+ }
+ }
+
+ public static class SseServerProperties {
+
+ private final String sseEndpoint;
+
+ private final String messageEndpoint;
+
+ public SseServerProperties(String sseEndpoint, String messageEndpoint) {
+ this.sseEndpoint = sseEndpoint;
+ this.messageEndpoint = messageEndpoint;
+ }
+ }
+
+ @PostConstruct
+ public void init() {
+ mcpType = env.getProperty("spring.ai.mcp.server.protocol", "sse");
+ if (mcpType.equals(STREAMABLE_TYPE)) {
+ String mcpEndPoint = env.getProperty("spring.ai.mcp.server.streamable-http.mcp-endpoint", "/mcp");
+ streamableProperties = new StreamableProperties(mcpEndPoint);
Review Comment:
The variable name 'mcpEndPoint' (line 93) uses inconsistent capitalization compared to the field name 'mcpEndpoint' (line 70). The parameter name should be 'mcpEndpoint' to maintain consistent naming throughout the codebase.
##########
console/src/main/java/org/apache/seata/console/filter/MCPAuthenticationFilter.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.console.filter;
+
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import org.apache.seata.console.security.User;
+import org.springframework.http.HttpStatus;
+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.AuthenticationException;
+import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.web.filter.OncePerRequestFilter;
+
+import java.io.IOException;
+
+public class MCPAuthenticationFilter extends OncePerRequestFilter {
+
+ private final AuthenticationManager authenticationManager;
+
+ /**
+ * Instantiates a new MCP authentication filter that authenticates requests
+ * using username and password provided in HTTP headers.
+ *
+ * @param authenticationManager the authentication manager used to validate credentials
+ */
+ public MCPAuthenticationFilter(AuthenticationManager authenticationManager) {
+ this.authenticationManager = authenticationManager;
+ }
+
+ @Override
+ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain chain)
+ throws IOException, ServletException {
+
+ Authentication existingAuth = SecurityContextHolder.getContext().getAuthentication();
+ if (existingAuth != null && existingAuth.isAuthenticated()) {
+ chain.doFilter(request, response);
+ return;
+ }
+
+ User user = resolveHeaders(request);
+
+ UsernamePasswordAuthenticationToken authenticationToken =
+ new UsernamePasswordAuthenticationToken(user.getUsername(), user.getPassword());
+
+ try {
+ Authentication authentication = authenticationManager.authenticate(authenticationToken);
Review Comment:
When password validation fails in CustomAuthenticationProvider, it returns a UsernamePasswordAuthenticationToken with null credentials and authorities, which is then treated as a successful authentication. In MCPAuthenticationFilter line 65, any non-null Authentication result from authenticationManager.authenticate() is stored in SecurityContext, treating it as successful. However, a failed password check should throw an AuthenticationException instead of returning a token. This creates a security vulnerability where any request with a valid username but invalid password would be authenticated.
```suggestion
Authentication authentication = authenticationManager.authenticate(authenticationToken);
if (authentication == null || !authentication.isAuthenticated()) {
throw new BadCredentialsException("Invalid credentials");
}
```
##########
console/src/main/java/org/apache/seata/console/filter/MCPAuthenticationFilter.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.console.filter;
+
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import org.apache.seata.console.security.User;
+import org.springframework.http.HttpStatus;
+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.AuthenticationException;
+import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.web.filter.OncePerRequestFilter;
+
+import java.io.IOException;
+
+public class MCPAuthenticationFilter extends OncePerRequestFilter {
+
+ private final AuthenticationManager authenticationManager;
+
+ /**
+ * Instantiates a new MCP authentication filter that authenticates requests
+ * using username and password provided in HTTP headers.
+ *
+ * @param authenticationManager the authentication manager used to validate credentials
+ */
+ public MCPAuthenticationFilter(AuthenticationManager authenticationManager) {
+ this.authenticationManager = authenticationManager;
+ }
+
+ @Override
+ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain chain)
+ throws IOException, ServletException {
+
+ Authentication existingAuth = SecurityContextHolder.getContext().getAuthentication();
+ if (existingAuth != null && existingAuth.isAuthenticated()) {
+ chain.doFilter(request, response);
+ return;
+ }
+
Review Comment:
The authentication check at line 54 allows already authenticated requests to bypass the authentication process, but this creates a security issue when combined with how MCP endpoints are configured. The WebSecurityConfig adds MCP endpoints to both the ignore list (lines 109-115) and csrf-ignore list (lines 126-132), which means these endpoints bypass Spring Security entirely. This makes the MCPAuthenticationFilter unreachable for MCP endpoint requests. Either remove MCP endpoints from the ignore list in WebSecurityConfig, or remove this early return check so authentication is always enforced when the filter is enabled.
```suggestion
```
##########
console/src/main/java/org/apache/seata/console/config/MCPFiltersConfig.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.console.config;
+
+import org.apache.seata.console.filter.MCPAuthenticationFilter;
+import org.apache.seata.mcp.core.props.MCPProperties;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.web.servlet.FilterRegistrationBean;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.core.Ordered;
+import org.springframework.security.authentication.AuthenticationManager;
+
+@Configuration
+public class MCPFiltersConfig {
+
+ @Autowired
+ private AuthenticationManager authenticationManager;
+
+ @Autowired
+ private MCPProperties mcpProperties;
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(MCPFiltersConfig.class);
+
+ @Bean
+ public FilterRegistrationBean<MCPAuthenticationFilter> mcpAuthenticationFilterRegistration() {
+
+ MCPAuthenticationFilter mcpAuthenticationFilter = new MCPAuthenticationFilter(authenticationManager);
+
+ FilterRegistrationBean<MCPAuthenticationFilter> registration = new FilterRegistrationBean<>();
+
+ registration.setFilter(mcpAuthenticationFilter);
+
+ for (String endPoint : mcpProperties.getEndpoints()) {
+ registration.addUrlPatterns(endPoint);
+ }
+
+ registration.setName("mcpJwtAuthenticationTokenFilter");
Review Comment:
The filter registration name 'mcpJwtAuthenticationTokenFilter' is misleading because this filter does not use JWT tokens. It uses basic username/password authentication from HTTP headers. The name should be 'mcpAuthenticationFilter' to accurately reflect its purpose and avoid confusion.
```suggestion
registration.setName("mcpAuthenticationFilter");
```
##########
console/src/main/java/org/apache/seata/console/config/WebSecurityConfig.java:
##########
@@ -98,7 +106,13 @@ public AuthenticationManager authenticationManager() {
@Bean
public WebSecurityCustomizer webSecurityCustomizer() {
- RequestMatcher[] ignoredMatchers = buildAntMatchers(env.getProperty("seata.security.ignore.urls", "/**"));
+ StringBuilder ignoreURLsBuilder = new StringBuilder(ignoreURLs);
+ List<String> mcpEndpoints = mcpProperties.getEndpoints();
+ for (String endpoint : mcpEndpoints) {
+ ignoreURLsBuilder.append(",").append(endpoint);
+ }
+ RequestMatcher[] ignoredMatchers =
+ buildAntMatchers(ignoreURLsBuilder.toString().trim());
Review Comment:
The MCP endpoints are being added to the security ignore list, which means Spring Security will completely bypass authentication for these endpoints. This conflicts with the purpose of MCPAuthenticationFilter which is meant to authenticate MCP requests. When endpoints are in the ignore list, they never reach any security filters. The MCPAuthenticationFilter registered in MCPFiltersConfig will not be invoked for these URLs. Remove the code that adds MCP endpoints to the ignore list if authentication is required for these endpoints.
```suggestion
RequestMatcher[] ignoredMatchers =
buildAntMatchers(ignoreURLs.trim());
```
##########
console/src/main/java/org/apache/seata/console/config/MCPFiltersConfig.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.console.config;
+
+import org.apache.seata.console.filter.MCPAuthenticationFilter;
+import org.apache.seata.mcp.core.props.MCPProperties;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.web.servlet.FilterRegistrationBean;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.core.Ordered;
+import org.springframework.security.authentication.AuthenticationManager;
+
+@Configuration
+public class MCPFiltersConfig {
+
+ @Autowired
+ private AuthenticationManager authenticationManager;
+
+ @Autowired
+ private MCPProperties mcpProperties;
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(MCPFiltersConfig.class);
+
+ @Bean
+ public FilterRegistrationBean<MCPAuthenticationFilter> mcpAuthenticationFilterRegistration() {
+
+ MCPAuthenticationFilter mcpAuthenticationFilter = new MCPAuthenticationFilter(authenticationManager);
+
+ FilterRegistrationBean<MCPAuthenticationFilter> registration = new FilterRegistrationBean<>();
+
+ registration.setFilter(mcpAuthenticationFilter);
+
+ for (String endPoint : mcpProperties.getEndpoints()) {
+ registration.addUrlPatterns(endPoint);
+ }
+
+ registration.setName("mcpJwtAuthenticationTokenFilter");
+
+ registration.setOrder(Ordered.HIGHEST_PRECEDENCE);
+
+ if (!mcpProperties.isEnableAuth())
+ LOGGER.warn(
+ "The AUTH VERIFICATION of the [MCP server] is not enabled, please ensure that it is enabled as much as possible to avoid security problems");
Review Comment:
The warning message suggests enabling authentication "as much as possible to avoid security problems", but this phrasing is unclear and unprofessional. The message should clearly state that disabling authentication creates a security risk. Consider rewording to: "MCP server authentication is disabled. This creates a security risk. It is strongly recommended to enable authentication by setting seata.mcp.auth.enabled=true"
```suggestion
"MCP server authentication is disabled. This creates a security risk. It is strongly recommended to enable authentication by setting seata.mcp.auth.enabled=true");
```
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: add MCP custom configuration and authentication code [incubator-seata]
Posted by "Xb2555 (via GitHub)" <gi...@apache.org>.
Xb2555 commented on PR #7876:
URL: https://github.com/apache/incubator-seata/pull/7876#issuecomment-3677690404
> Has the previous MCP-related content been merged, or should we merge this one first?
Previously, only the dependencies related to MCP were introduced. Now, this PR introduces the MCP security configuration and custom parameter configuration. It can be directly merged in without any impact.
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: add MCP custom configuration and authentication code [incubator-seata]
Posted by "xjlgod (via GitHub)" <gi...@apache.org>.
xjlgod commented on code in PR #7876:
URL: https://github.com/apache/incubator-seata/pull/7876#discussion_r2636961104
##########
console/src/main/java/org/apache/seata/console/config/WebSecurityConfig.java:
##########
@@ -98,7 +103,13 @@ public AuthenticationManager authenticationManager() {
@Bean
public WebSecurityCustomizer webSecurityCustomizer() {
- RequestMatcher[] ignoredMatchers = buildAntMatchers(env.getProperty("seata.security.ignore.urls", "/**"));
+ StringBuilder ignoreURLs = new StringBuilder(env.getProperty("seata.security.ignore.urls", "/**"));
Review Comment:
maybe we can use string joiner
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: add MCP custom configuration and authentication code [incubator-seata]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #7876:
URL: https://github.com/apache/incubator-seata/pull/7876#issuecomment-3674474315
## [Codecov](https://app.codecov.io/gh/apache/incubator-seata/pull/7876?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 71.07%. Comparing base ([`8629d18`](https://app.codecov.io/gh/apache/incubator-seata/commit/8629d183d072508980bdf37c229cd8c12dddfdf6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`c8e3914`](https://app.codecov.io/gh/apache/incubator-seata/commit/c8e3914ad594d6e3c088f42a3e962d4f05234bee?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 2.x #7876 +/- ##
============================================
- Coverage 71.10% 71.07% -0.03%
Complexity 797 797
============================================
Files 1294 1294
Lines 49528 49528
Branches 5871 5871
============================================
- Hits 35216 35204 -12
- Misses 11403 11417 +14
+ Partials 2909 2907 -2
```
[see 8 files with indirect coverage changes](https://app.codecov.io/gh/apache/incubator-seata/pull/7876/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
[](https://app.codecov.io/gh/apache/incubator-seata/pull/7876?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: add MCP custom configuration and authentication code [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes merged PR #7876:
URL: https://github.com/apache/incubator-seata/pull/7876
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: add MCP custom configuration and authentication code [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on code in PR #7876:
URL: https://github.com/apache/incubator-seata/pull/7876#discussion_r2639448506
##########
console/src/main/java/org/apache/seata/console/config/WebSecurityConfig.java:
##########
@@ -109,8 +120,13 @@ public WebSecurityCustomizer webSecurityCustomizer() {
@Bean
public SecurityFilterChain securityFilterChain(HttpSecurity http, AuthenticationManager authenticationManager)
throws Exception {
- RequestMatcher[] csrfIgnored = buildAntMatchers(env.getProperty("seata.security.csrf-ignore-urls"));
-
+ StringBuilder csrfIgnoreUrls = new StringBuilder(env.getProperty("seata.security.csrf-ignore-urls", "/**"));
Review Comment:
why not use @value?
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: add MCP custom configuration and authentication code [incubator-seata]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #7876:
URL: https://github.com/apache/incubator-seata/pull/7876#discussion_r2642152724
##########
console/src/main/java/org/apache/seata/console/config/WebSecurityConfig.java:
##########
@@ -98,7 +106,13 @@ public AuthenticationManager authenticationManager() {
@Bean
public WebSecurityCustomizer webSecurityCustomizer() {
- RequestMatcher[] ignoredMatchers = buildAntMatchers(env.getProperty("seata.security.ignore.urls", "/**"));
+ StringBuilder ignoreURLsBuilder = new StringBuilder(ignoreURLs);
+ List<String> mcpEndpoints = mcpProperties.getEndpoints();
+ for (String endpoint : mcpEndpoints) {
+ ignoreURLsBuilder.append(",").append(endpoint);
Review Comment:
The security approach of bypassing authentication for MCP endpoints by adding them to the ignore list is problematic. The MCPProperties class has an `isEnableAuth()` method, but it's not being used here. The code unconditionally adds MCP endpoints to the ignore list (bypassing authentication), regardless of whether `seata.mcp.auth.enabled` is true or false. This means authentication cannot be enforced for MCP endpoints even when configured to do so. Consider checking `mcpProperties.isEnableAuth()` before adding endpoints to the ignore list.
```suggestion
if (!mcpProperties.isEnableAuth() && mcpEndpoints != null) {
for (String endpoint : mcpEndpoints) {
ignoreURLsBuilder.append(",").append(endpoint);
}
```
##########
console/src/main/java/org/apache/seata/console/config/WebSecurityConfig.java:
##########
@@ -109,8 +123,13 @@ public WebSecurityCustomizer webSecurityCustomizer() {
@Bean
public SecurityFilterChain securityFilterChain(HttpSecurity http, AuthenticationManager authenticationManager)
throws Exception {
- RequestMatcher[] csrfIgnored = buildAntMatchers(env.getProperty("seata.security.csrf-ignore-urls"));
-
+ StringBuilder csrfIgnoreUrlsBuilder = new StringBuilder(csrfIgnoreUrls);
+ List<String> mcpEndpoints = mcpProperties.getEndpoints();
+ for (String endpoint : mcpEndpoints) {
+ csrfIgnoreUrlsBuilder.append(",").append(endpoint);
Review Comment:
The same security issue exists here with CSRF protection. The MCP endpoints are unconditionally added to the CSRF ignore list regardless of the `seata.mcp.auth.enabled` configuration. This could expose MCP endpoints to CSRF attacks even when authentication is intended to be enabled. Consider checking `mcpProperties.isEnableAuth()` before adding endpoints to the CSRF ignore list.
```suggestion
if (!mcpProperties.isEnableAuth()) {
for (String endpoint : mcpEndpoints) {
csrfIgnoreUrlsBuilder.append(",").append(endpoint);
}
```
##########
console/src/main/java/org/apache/seata/mcp/core/props/MCPProperties.java:
##########
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.mcp.core.props;
+
+import jakarta.annotation.Nullable;
+import jakarta.annotation.PostConstruct;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.ai.mcp.server.common.autoconfigure.properties.McpServerProperties;
+import org.springframework.ai.mcp.server.common.autoconfigure.properties.McpServerSseProperties;
+import org.springframework.ai.mcp.server.common.autoconfigure.properties.McpServerStreamableHttpProperties;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.core.env.Environment;
+import org.springframework.stereotype.Component;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+@Component
+public class MCPProperties {
+
+ private final Environment env;
+
+ private boolean enableAuth = true;
+
+ private Long queryDuration = TimeUnit.DAYS.toMillis(1);
+
+ private final McpServerProperties mcpServerProperties;
+
+ private final McpServerSseProperties mcpServerSseProperties;
+
+ private final McpServerStreamableHttpProperties mcpServerStreamableHttpProperties;
+
+ private final List<String> endpoints = new ArrayList<>();
+
+ private final Logger logger = LoggerFactory.getLogger(MCPProperties.class);
+
+ @Autowired
+ public MCPProperties(
+ @Nullable McpServerProperties mcpServerProperties,
+ Environment env,
+ @Nullable McpServerSseProperties serverSseProperties,
+ @Nullable McpServerStreamableHttpProperties serverStreamableHttpProperties) {
+ this.mcpServerProperties = mcpServerProperties;
+ this.env = env;
+ this.mcpServerSseProperties = serverSseProperties;
+ this.mcpServerStreamableHttpProperties = serverStreamableHttpProperties;
+ }
+
+ public List<String> getEndpoints() {
+ return Collections.unmodifiableList(new ArrayList<>(endpoints));
Review Comment:
The method creates a defensive copy by wrapping the endpoints list in an unmodifiable list, but then creates another ArrayList copy. The double copy (new ArrayList inside unmodifiableList) is redundant. Since Collections.unmodifiableList already protects against modifications, the inner ArrayList copy is unnecessary. Simply return Collections.unmodifiableList(endpoints) to maintain immutability while avoiding the unnecessary object allocation.
```suggestion
return Collections.unmodifiableList(endpoints);
```
##########
console/src/main/java/org/apache/seata/mcp/core/props/MCPProperties.java:
##########
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.mcp.core.props;
+
+import jakarta.annotation.Nullable;
+import jakarta.annotation.PostConstruct;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.ai.mcp.server.common.autoconfigure.properties.McpServerProperties;
+import org.springframework.ai.mcp.server.common.autoconfigure.properties.McpServerSseProperties;
+import org.springframework.ai.mcp.server.common.autoconfigure.properties.McpServerStreamableHttpProperties;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.core.env.Environment;
+import org.springframework.stereotype.Component;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+@Component
+public class MCPProperties {
+
+ private final Environment env;
+
+ private boolean enableAuth = true;
+
+ private Long queryDuration = TimeUnit.DAYS.toMillis(1);
+
+ private final McpServerProperties mcpServerProperties;
+
+ private final McpServerSseProperties mcpServerSseProperties;
+
+ private final McpServerStreamableHttpProperties mcpServerStreamableHttpProperties;
+
+ private final List<String> endpoints = new ArrayList<>();
+
+ private final Logger logger = LoggerFactory.getLogger(MCPProperties.class);
+
+ @Autowired
+ public MCPProperties(
+ @Nullable McpServerProperties mcpServerProperties,
+ Environment env,
+ @Nullable McpServerSseProperties serverSseProperties,
+ @Nullable McpServerStreamableHttpProperties serverStreamableHttpProperties) {
+ this.mcpServerProperties = mcpServerProperties;
+ this.env = env;
+ this.mcpServerSseProperties = serverSseProperties;
+ this.mcpServerStreamableHttpProperties = serverStreamableHttpProperties;
+ }
+
+ public List<String> getEndpoints() {
+ return Collections.unmodifiableList(new ArrayList<>(endpoints));
+ }
+
+ public Long getQueryDuration() {
+ return queryDuration;
+ }
+
+ @PostConstruct
+ public void init() {
+ String maxQueryDurationStr = env.getProperty("seata.mcp.query.max-query-duration", "86400000");
+ try {
+ queryDuration = Long.parseLong(maxQueryDurationStr);
+ } catch (NumberFormatException ex) {
+ queryDuration = TimeUnit.DAYS.toMillis(1);
+ }
+ enableAuth = Boolean.parseBoolean(env.getProperty("seata.mcp.auth.enabled", "true"));
+
+ if (!enableAuth) {
+ logger.warn(
+ "MCP server authentication is disabled. This creates a security risk. It is strongly recommended to enable authentication by setting seata.mcp.auth.enabled=true");
+ }
+
+ if (mcpServerProperties != null) {
+ McpServerProperties.ServerProtocol protocol = mcpServerProperties.getProtocol();
+ if (protocol == McpServerProperties.ServerProtocol.SSE && mcpServerSseProperties != null) {
+ endpoints.add(mcpServerSseProperties.getSseEndpoint());
+ endpoints.add(mcpServerSseProperties.getSseMessageEndpoint());
+ } else if (protocol == McpServerProperties.ServerProtocol.STREAMABLE
+ && mcpServerStreamableHttpProperties != null) {
+ endpoints.add(mcpServerStreamableHttpProperties.getMcpEndpoint());
+ } else {
+ throw new IllegalStateException(
+ "MCP server properties not properly configured or unsupported protocol");
+ }
+ } else {
+ logger.warn("MCP server properties not properly configured");
Review Comment:
The error handling in the else block silently logs a warning and continues when the protocol is unsupported or properties are null. This could lead to the endpoints list being empty, which might cause the application to behave unexpectedly. Consider throwing the IllegalStateException in all cases where the configuration is invalid, not just when protocol properties don't match the configured protocol.
```suggestion
throw new IllegalStateException("MCP server properties not properly configured");
```
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: add MCP custom configuration and authentication code [incubator-seata]
Posted by "xjlgod (via GitHub)" <gi...@apache.org>.
xjlgod commented on PR #7876:
URL: https://github.com/apache/incubator-seata/pull/7876#issuecomment-3677643910
Has the previous MCP-related content been merged, or should we merge this one first?
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: add MCP custom configuration and authentication code [incubator-seata]
Posted by "Xb2555 (via GitHub)" <gi...@apache.org>.
Xb2555 commented on code in PR #7876:
URL: https://github.com/apache/incubator-seata/pull/7876#discussion_r2637023460
##########
console/src/main/java/org/apache/seata/console/config/WebSecurityConfig.java:
##########
@@ -98,7 +103,13 @@ public AuthenticationManager authenticationManager() {
@Bean
public WebSecurityCustomizer webSecurityCustomizer() {
- RequestMatcher[] ignoredMatchers = buildAntMatchers(env.getProperty("seata.security.ignore.urls", "/**"));
+ StringBuilder ignoreURLs = new StringBuilder(env.getProperty("seata.security.ignore.urls", "/**"));
Review Comment:
I tried before. The path that needs to be ignored directly passed in as a string cannot be configured correctly. It needs to be converted to the RequestMatcher array (it might be a problem with the new version of security).
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: add MCP custom configuration and authentication code [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on code in PR #7876:
URL: https://github.com/apache/incubator-seata/pull/7876#discussion_r2639464947
##########
console/src/main/java/org/apache/seata/console/filter/MCPAuthenticationFilter.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.console.filter;
+
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import org.apache.seata.console.security.User;
+import org.springframework.http.HttpStatus;
+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.AuthenticationException;
+import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.web.filter.OncePerRequestFilter;
+
+import java.io.IOException;
+
+public class MCPAuthenticationFilter extends OncePerRequestFilter {
+
+ private final AuthenticationManager authenticationManager;
+
+ /**
+ * Instantiates a new MCP authentication filter that authenticates requests
+ * using username and password provided in HTTP headers.
+ *
+ * @param authenticationManager the authentication manager used to validate credentials
+ */
+ public MCPAuthenticationFilter(AuthenticationManager authenticationManager) {
+ this.authenticationManager = authenticationManager;
+ }
+
+ @Override
+ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain chain)
+ throws IOException, ServletException {
+
+ Authentication existingAuth = SecurityContextHolder.getContext().getAuthentication();
+ if (existingAuth != null && existingAuth.isAuthenticated()) {
+ chain.doFilter(request, response);
+ return;
+ }
+
+ User user = resolveHeaders(request);
+
+ UsernamePasswordAuthenticationToken authenticationToken =
+ new UsernamePasswordAuthenticationToken(user.getUsername(), user.getPassword());
+
+ try {
+ Authentication authentication = authenticationManager.authenticate(authenticationToken);
Review Comment:
Why do we need a separate authentication step? Why not just follow the standard flow of obtaining a token through the login interface?
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: add MCP custom configuration and authentication code [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on code in PR #7876:
URL: https://github.com/apache/incubator-seata/pull/7876#discussion_r2639471398
##########
console/src/main/java/org/apache/seata/mcp/core/props/MCPProperties.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.mcp.core.props;
+
+import jakarta.annotation.PostConstruct;
+import org.springframework.core.env.Environment;
+import org.springframework.stereotype.Component;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+@Component
+public class MCPProperties {
+
+ private final Environment env;
+
+ public static final String SSE_TYPE = "sse";
+
+ public static final String STREAMABLE_TYPE = "streamable";
+
+ private boolean enableAuth = true;
+
+ private Long queryDuration = TimeUnit.DAYS.toMillis(1);
+
+ private String mcpType = SSE_TYPE;
+
+ private StreamableProperties streamableProperties;
+
+ private SseServerProperties sseServerProperties;
+
+ public MCPProperties(Environment env) {
+ this.env = env;
+ }
+
+ public boolean isSseType() {
+ return mcpType.equals(SSE_TYPE);
+ }
+
+ public List<String> getEndpoints() {
+ List<String> result = new ArrayList<>();
+ if (isSseType()) {
+ result.add(sseServerProperties.sseEndpoint);
+ result.add(sseServerProperties.messageEndpoint);
+ } else {
+ result.add(streamableProperties.mcpEndpoint);
+ }
+ return result;
+ }
+
+ public Long getQueryDuration() {
+ return queryDuration;
+ }
+
+ public static class StreamableProperties {
+ private final String mcpEndpoint;
+
+ public StreamableProperties(String mcpEndPoint) {
+ this.mcpEndpoint = mcpEndPoint;
+ }
+ }
+
+ public static class SseServerProperties {
+
+ private final String sseEndpoint;
+
+ private final String messageEndpoint;
+
+ public SseServerProperties(String sseEndpoint, String messageEndpoint) {
+ this.sseEndpoint = sseEndpoint;
+ this.messageEndpoint = messageEndpoint;
+ }
+ }
+
+ @PostConstruct
+ public void init() {
+ mcpType = env.getProperty("spring.ai.mcp.server.protocol", "sse");
+ if (mcpType.equals(STREAMABLE_TYPE)) {
+ String mcpEndPoint = env.getProperty("spring.ai.mcp.server.streamable-http.mcp-endpoint", "/mcp");
+ streamableProperties = new StreamableProperties(mcpEndPoint);
+ } else {
+ mcpType = SSE_TYPE;
+ String sseEndpoint = env.getProperty("spring.ai.mcp.server.sse-endpoint", "/sse");
Review Comment:
Doesn't this parameter have a corresponding properties bean?
##########
console/src/main/java/org/apache/seata/console/filter/MCPAuthenticationFilter.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.console.filter;
+
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import org.apache.seata.console.security.User;
+import org.springframework.http.HttpStatus;
+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.AuthenticationException;
+import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.web.filter.OncePerRequestFilter;
+
+import java.io.IOException;
+
+public class MCPAuthenticationFilter extends OncePerRequestFilter {
+
+ private final AuthenticationManager authenticationManager;
+
+ /**
+ * Instantiates a new MCP authentication filter that authenticates requests
+ * using username and password provided in HTTP headers.
+ *
+ * @param authenticationManager the authentication manager used to validate credentials
+ */
+ public MCPAuthenticationFilter(AuthenticationManager authenticationManager) {
+ this.authenticationManager = authenticationManager;
+ }
+
+ @Override
+ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain chain)
+ throws IOException, ServletException {
+
+ Authentication existingAuth = SecurityContextHolder.getContext().getAuthentication();
+ if (existingAuth != null && existingAuth.isAuthenticated()) {
+ chain.doFilter(request, response);
+ return;
+ }
+
+ User user = resolveHeaders(request);
+
+ UsernamePasswordAuthenticationToken authenticationToken =
+ new UsernamePasswordAuthenticationToken(user.getUsername(), user.getPassword());
+
+ try {
+ Authentication authentication = authenticationManager.authenticate(authenticationToken);
+ SecurityContextHolder.getContext().setAuthentication(authentication);
+ } catch (AuthenticationException e) {
+ response.sendError(HttpStatus.UNAUTHORIZED.value(), "Invalid credentials");
+ return;
+ }
+
+ chain.doFilter(request, response);
+ }
+
+ /**
+ * Get userDetails from header
+ */
+ private User resolveHeaders(HttpServletRequest request) {
+ String username = request.getHeader("X-Mcp-Username");
Review Comment:
Is this a standard MCP HTTP header? If not, I wouldn't recommend having two separate authentication logics—we should just reuse the existing authentication logic for verification instead.
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org