Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new openam-mcp-server Spring Boot module to the OpenAM reactor, providing an MCP (Model Context Protocol) management server that can manage users/realms and read auth module/chain configuration from OpenAM (with optional OAuth2-based authentication), plus initial unit tests and test fixtures.
Changes:
- Adds a JDK17-activated Maven reactor profile to include the new
openam-mcp-servermodule. - Implements MCP tools/services for users, realms, and authentication configuration, plus request authentication via a Spring MVC interceptor.
- Adds basic Spring Boot + service-level tests and JSON fixtures for OpenAM REST responses.
Reviewed changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated 27 comments.
Show a summary per file
| File | Description |
|---|---|
| pom.xml | Adds a JDK17-activated profile to include openam-mcp-server in the reactor build. |
| openam-mcp-server/pom.xml | New Maven module with Spring Boot + Spring AI MCP server dependencies and build config. |
| openam-mcp-server/README.md | Module documentation, setup instructions, and tool list/examples. |
| openam-mcp-server/src/main/resources/application.yml | Default configuration for the MCP server and OpenAM connection/auth settings. |
| openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/OpenAmMcpServerApplication.java | Spring Boot app entrypoint; RestClient + tool registration beans. |
| openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/config/OpenAMConfig.java | @ConfigurationProperties record for OpenAM connection/auth settings. |
| openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/config/WebConfig.java | Registers the authentication interceptor for /mcp/**. |
| openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/controller/OAuth2Controller.java | Proxies OpenAM OAuth2 .well-known endpoints through this server. |
| openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/security/AuthInterceptor.java | Auth flow (username/password or OAuth2), token caching, and request token propagation. |
| openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/service/OpenAMAbstractService.java | Base service logic for retrieving the request token and default realm. |
| openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/service/UserService.java | MCP tools for listing users, setting attributes/passwords, creating and deleting users. |
| openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/service/RealmService.java | MCP tool for listing realms. |
| openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/service/AuthenticationConfigService.java | MCP tools for listing auth modules, chains, and available module types; schema/settings mapping. |
| openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/model/SearchResponseDTO.java | DTO for OpenAM list/search responses. |
| openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/model/UserDTO.java | DTO for OpenAM user JSON. |
| openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/model/User.java | Public user model returned by MCP tools. |
| openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/model/RealmDTO.java | DTO for OpenAM realm JSON. |
| openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/model/Realm.java | Public realm model returned by MCP tools. |
| openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/model/PropertySchemaDTO.java | DTO for module schema property metadata. |
| openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/model/CoreAuthModuleDTO.java | DTO for “all available auth module types” response. |
| openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/model/CoreAuthModule.java | Public model for available auth module types. |
| openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/model/AuthModuleSchemaDTO.java | DTO for module schema response. |
| openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/model/AuthModuleDTO.java | DTO for realm auth module instances. |
| openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/model/AuthModule.java | Public auth module model with settings map. |
| openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/model/AuthChainDTO.java | DTO for auth chain response. |
| openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/model/AuthChain.java | Public auth chain model with resolved module names. |
| openam-mcp-server/src/test/java/org/openidentityplatform/openam/mcp/server/OpenAmMcpServerApplicationTests.java | Spring context smoke test. |
| openam-mcp-server/src/test/java/org/openidentityplatform/openam/mcp/server/service/OpenAMServiceTest.java | Shared RestClient/RequestContextHolder mocking setup for service tests. |
| openam-mcp-server/src/test/java/org/openidentityplatform/openam/mcp/server/service/UserServiceTest.java | Unit tests for user-related tool methods. |
| openam-mcp-server/src/test/java/org/openidentityplatform/openam/mcp/server/service/RealmServiceTest.java | Unit test for realm listing. |
| openam-mcp-server/src/test/java/org/openidentityplatform/openam/mcp/server/service/AuthenticationConfigServiceTest.java | Unit tests for auth chain/module listing. |
| openam-mcp-server/src/test/resources/users/users-list-response.json | Test fixture for OpenAM user list response. |
| openam-mcp-server/src/test/resources/users/user-response.json | Test fixture for OpenAM user response. |
| openam-mcp-server/src/test/resources/realms/realms-response.json | Test fixture for OpenAM realm list response. |
| openam-mcp-server/src/test/resources/auth/modules-response.json | Test fixture for realm auth module instances response. |
| openam-mcp-server/src/test/resources/auth/module-settings-response.json | Test fixture for module settings response. |
| openam-mcp-server/src/test/resources/auth/module-schema-response.json | Test fixture for module schema response. |
| openam-mcp-server/src/test/resources/auth/chains-response.json | Test fixture for realm auth chains response. |
| openam-mcp-server/src/test/resources/auth/all-modules-response.json | Test fixture for “all available auth module types” response. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @Override | ||
| public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception { | ||
| if(openAMConfig.useOAuthForAuthentication()) { | ||
| return preHandleOAuth(request, response); | ||
| } else { | ||
| return preHandleUsernamePassword(request); | ||
| } | ||
| } |
There was a problem hiding this comment.
AuthInterceptor contains critical authentication logic (OAuth vs username/password, token caching/refresh, error handling) but there are no tests validating the main branches (missing/invalid Authorization header, expired token refresh, etc.). Adding unit tests around preHandle* would help prevent regressions and avoid infinite-recursion scenarios.
...ain/java/org/openidentityplatform/openam/mcp/server/service/AuthenticationConfigService.java
Outdated
Show resolved
Hide resolved
| long seconds = tokenValidSeconds(token); | ||
| if(seconds > 1) { | ||
| request.setAttribute("tokenId", token); | ||
| return true; | ||
| } | ||
| log.info("preHandleUsernamePassword: token {} is about to expire in {} s", token, seconds); | ||
| tokenCache.invalidate(LOGIN_PASSWORD_TOKEN_KEY); | ||
| return preHandleUsernamePassword(request); | ||
| } |
There was a problem hiding this comment.
preHandleUsernamePassword uses recursion to refresh the token. If tokenValidSeconds() keeps returning <= 1 (e.g., because the session endpoint is down or the expiry value can't be parsed), this will recurse until stack overflow. Replace the recursion with a bounded retry loop (and fail fast with a clear error if a fresh token still can't be validated).
...erver/src/test/java/org/openidentityplatform/openam/mcp/server/service/RealmServiceTest.java
Outdated
Show resolved
Hide resolved
| @Tool(name = "create_user", description = "Creates a new user") | ||
| public User createUser(@ToolParam(required = false, description = "If not set, uses root realm") String realm, | ||
| @ToolParam(description = "Username (login)") String userName, | ||
| @ToolParam(description = "Password (min length 8)") String password, | ||
| @ToolParam(required = false, description = "User family name") String familyName, | ||
| @ToolParam(required = false, description = "User given name") String givenName, | ||
| @ToolParam(required = false, description = "Name") String name, | ||
| @ToolParam(required = false, description = "Email") String mail, | ||
| @ToolParam(required = false, description = "Phone number") String phone |
There was a problem hiding this comment.
createUser(...) adds non-trivial request-body construction + a POST to OpenAM, but there is no unit test covering it (unlike the other UserService tool methods). Adding a test for the payload/URI (including optional null fields) would help prevent regressions.
| username: ${OPENAM_ADMIN_USERNAME:amadmin} | ||
| password: ${OPENAM_ADMIN_PASSWORD:ampassword} |
There was a problem hiding this comment.
openam.username and especially openam.password have insecure defaults (amadmin / ampassword). This makes it easy to start the service with weak credentials by accident. Prefer no default (fail fast if unset) or use a clearly-invalid placeholder that forces configuration via environment/secret management.
| username: ${OPENAM_ADMIN_USERNAME:amadmin} | |
| password: ${OPENAM_ADMIN_PASSWORD:ampassword} | |
| username: ${OPENAM_ADMIN_USERNAME:CHANGE_ME_OPENAM_ADMIN_USERNAME} | |
| password: ${OPENAM_ADMIN_PASSWORD:CHANGE_ME_OPENAM_ADMIN_PASSWORD} |
| if (!accessTokenValid(accessToken)) { | ||
| response.setStatus(HttpStatus.UNAUTHORIZED.value()); | ||
| response.setHeader("WWW-Authenticate", "Bearer realm=\"OpenAM\""); | ||
| response.setContentType("application/json"); | ||
| response.getWriter().write("{\"error\":\"Unauthorized\"}"); | ||
| return false; | ||
| } | ||
| final String token; | ||
| try { | ||
| token = tokenCache.get(accessToken, this::getTokenIdFromAccessToken); | ||
| } catch (Exception e) { | ||
| log.warn("error getting token:", e); | ||
| throw e; | ||
| } |
There was a problem hiding this comment.
preHandleOAuth calls accessTokenValid() (userinfo request) on every incoming request, even when the access token is already cached in tokenCache. This adds an extra network round-trip per call and can become a throughput bottleneck. Consider caching the validation result (or relying on the token->session exchange result) with an expiry aligned to the access token lifetime.
| if (!accessTokenValid(accessToken)) { | |
| response.setStatus(HttpStatus.UNAUTHORIZED.value()); | |
| response.setHeader("WWW-Authenticate", "Bearer realm=\"OpenAM\""); | |
| response.setContentType("application/json"); | |
| response.getWriter().write("{\"error\":\"Unauthorized\"}"); | |
| return false; | |
| } | |
| final String token; | |
| try { | |
| token = tokenCache.get(accessToken, this::getTokenIdFromAccessToken); | |
| } catch (Exception e) { | |
| log.warn("error getting token:", e); | |
| throw e; | |
| } | |
| final String token; | |
| try { | |
| token = tokenCache.get(accessToken, this::getTokenIdFromAccessToken); | |
| } catch (Exception e) { | |
| log.warn("preHandleOAuth: error getting token for access token {}: {}", accessToken, e.getMessage()); | |
| response.setStatus(HttpStatus.UNAUTHORIZED.value()); | |
| response.setHeader("WWW-Authenticate", "Bearer realm=\"OpenAM\""); | |
| response.setContentType("application/json"); | |
| response.getWriter().write("{\"error\":\"Unauthorized\"}"); | |
| return false; | |
| } |
...er/src/main/java/org/openidentityplatform/openam/mcp/server/controller/OAuth2Controller.java
Outdated
Show resolved
Hide resolved
openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/model/Realm.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
OpenAM MCP Server is a lightweight management service for OpenAM user accounts. It allows administrators to create, update, delete, and reset passwords for users, as well as retrieve authentication modules and chains configurations.