Copilot commented on code in PR #10971: URL: https://github.com/apache/gravitino/pull/10971#discussion_r3212862660
########## core/src/main/java/org/apache/gravitino/authorization/IdpGroupManager.java: ########## @@ -0,0 +1,236 @@ +/* + * 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.gravitino.authorization; + +import com.google.common.base.Preconditions; +import java.util.ArrayList; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; +import org.apache.commons.lang3.StringUtils; +import org.apache.gravitino.Config; +import org.apache.gravitino.Configs; +import org.apache.gravitino.GravitinoEnv; +import org.apache.gravitino.dto.IdpGroupDTO; +import org.apache.gravitino.exceptions.ForbiddenException; +import org.apache.gravitino.exceptions.GroupAlreadyExistsException; +import org.apache.gravitino.exceptions.NoSuchGroupException; +import org.apache.gravitino.exceptions.NoSuchUserException; +import org.apache.gravitino.storage.IdGenerator; +import org.apache.gravitino.storage.relational.po.IdpGroupPO; +import org.apache.gravitino.storage.relational.po.IdpGroupUserRelPO; +import org.apache.gravitino.storage.relational.po.IdpUserPO; +import org.apache.gravitino.storage.relational.service.IdpGroupMetaService; +import org.apache.gravitino.storage.relational.service.IdpUserMetaService; +import org.apache.gravitino.utils.PrincipalUtils; + +/** Manager for built-in IdP group management APIs. */ +public class IdpGroupManager { + private static final long INITIAL_VERSION = 1L; + + private final Config config; + private final IdGenerator idGenerator; + private final IdpUserMetaService userMetaService; + private final IdpGroupMetaService groupMetaService; + + public static IdpGroupManager fromEnvironment() { + return new IdpGroupManager( + GravitinoEnv.getInstance().config(), + GravitinoEnv.getInstance().idGenerator(), + IdpUserMetaService.getInstance(), + IdpGroupMetaService.getInstance()); + } + + IdpGroupManager( + Config config, + IdGenerator idGenerator, + IdpUserMetaService userMetaService, + IdpGroupMetaService groupMetaService) { + this.config = config; + this.idGenerator = idGenerator; + this.userMetaService = userMetaService; + this.groupMetaService = groupMetaService; + } + + public IdpGroupDTO createGroup(String groupName) { + ensureServiceAdmin(); + validateGroupName(groupName); + if (groupMetaService().findGroup(groupName).isPresent()) { + throw new GroupAlreadyExistsException("Built-in IdP group %s already exists", groupName); + } + + groupMetaService() + .createGroup( + IdpGroupPO.builder() + .withGroupId(nextId()) + .withGroupName(groupName) + .withCurrentVersion(INITIAL_VERSION) + .withLastVersion(INITIAL_VERSION) + .withDeletedAt(0L) + .build()); + return getGroup(groupName); + } + + public IdpGroupDTO getGroup(String groupName) { + validateGroupName(groupName); + IdpGroupPO groupPO = + groupMetaService() + .findGroup(groupName) + .orElseThrow( + () -> new NoSuchGroupException("Built-in IdP group %s does not exist", groupName)); + return toGroupDTO(groupPO); + } Review Comment: `getGroup` does not call `ensureServiceAdmin()`, which means built-in IdP group membership can be queried by any caller through the REST API. This exposes potentially sensitive identity/group info and enables group/user enumeration. Consider enforcing service-admin access for reads (or document and implement a narrower access policy) so read APIs are consistent with the write-side admin enforcement. ########## common/src/main/java/org/apache/gravitino/dto/responses/IdpGroupResponse.java: ########## @@ -0,0 +1,68 @@ +/* + * 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.gravitino.dto.responses; + +import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Preconditions; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.ToString; +import org.apache.commons.lang3.StringUtils; +import org.apache.gravitino.dto.IdpGroupDTO; + +/** Represents a response for a built-in IdP group. */ +@Getter +@ToString +@EqualsAndHashCode(callSuper = true) +public class IdpGroupResponse extends BaseResponse { + + @JsonProperty("group") + private final IdpGroupDTO group; + + /** + * Constructor for IdpGroupResponse. + * + * @param group The built-in IdP group data transfer object. + */ + public IdpGroupResponse(IdpGroupDTO group) { + super(0); + this.group = group; + } + + /** Default constructor for IdpGroupResponse. (Used for Jackson deserialization.) */ + public IdpGroupResponse() { + super(); + this.group = null; + } + + /** + * Validates the response data. + * + * @throws IllegalArgumentException if the name is not set. + */ + @Override + public void validate() throws IllegalArgumentException { + super.validate(); + + Preconditions.checkArgument(group != null, "group must not be null"); + Preconditions.checkArgument( + StringUtils.isNotBlank(group.name()), "group 'name' must not be null and empty"); Review Comment: Spelling/grammar: the validation message says "must not be null and empty" but the check is for blank (null *or* empty/whitespace). Consider changing the message to "must not be null or empty" for clarity. ########## common/src/main/java/org/apache/gravitino/dto/responses/IdpUserResponse.java: ########## @@ -0,0 +1,68 @@ +/* + * 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.gravitino.dto.responses; + +import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Preconditions; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.ToString; +import org.apache.commons.lang3.StringUtils; +import org.apache.gravitino.dto.IdpUserDTO; + +/** Represents a response for a built-in IdP user. */ +@Getter +@ToString +@EqualsAndHashCode(callSuper = true) +public class IdpUserResponse extends BaseResponse { + + @JsonProperty("user") + private final IdpUserDTO user; + + /** + * Constructor for IdpUserResponse. + * + * @param user The built-in IdP user data transfer object. + */ + public IdpUserResponse(IdpUserDTO user) { + super(0); + this.user = user; + } + + /** Default constructor for IdpUserResponse. (Used for Jackson deserialization.) */ + public IdpUserResponse() { + super(); + this.user = null; + } + + /** + * Validates the response data. + * + * @throws IllegalArgumentException if the name is not set. + */ + @Override + public void validate() throws IllegalArgumentException { + super.validate(); + + Preconditions.checkArgument(user != null, "user must not be null"); + Preconditions.checkArgument( + StringUtils.isNotBlank(user.name()), "user 'name' must not be null and empty"); Review Comment: Spelling/grammar: the validation message says "must not be null and empty" but the check is for blank (null *or* empty/whitespace). Consider changing the message to "must not be null or empty" for clarity. ########## server/src/test/java/org/apache/gravitino/server/web/rest/TestIdpUserOperations.java: ########## @@ -0,0 +1,396 @@ +/* + * 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.gravitino.server.web.rest; + +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import javax.servlet.http.HttpServletRequest; +import javax.ws.rs.client.Entity; +import javax.ws.rs.core.Application; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import org.apache.gravitino.authorization.IdpUserManager; +import org.apache.gravitino.dto.IdpUserDTO; +import org.apache.gravitino.dto.requests.CreateUserRequest; +import org.apache.gravitino.dto.requests.ResetPasswordRequest; +import org.apache.gravitino.dto.responses.ErrorConstants; +import org.apache.gravitino.dto.responses.ErrorResponse; +import org.apache.gravitino.dto.responses.IdpUserResponse; +import org.apache.gravitino.dto.responses.RemoveResponse; +import org.apache.gravitino.exceptions.ForbiddenException; +import org.apache.gravitino.exceptions.NoSuchUserException; +import org.apache.gravitino.exceptions.UserAlreadyExistsException; +import org.apache.gravitino.rest.RESTUtils; +import org.glassfish.hk2.utilities.binding.AbstractBinder; +import org.glassfish.jersey.server.ResourceConfig; +import org.glassfish.jersey.test.TestProperties; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class TestIdpUserOperations extends BaseOperationsTest { + + private static final IdpUserManager MANAGER = mock(IdpUserManager.class); + + public static class TestableIdpUserOperations extends IdpUserOperations { + public TestableIdpUserOperations() { + super(MANAGER); + } + } + + private static class MockServletRequestFactory extends ServletRequestFactoryBase { + @Override + public HttpServletRequest get() { + HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getRemoteUser()).thenReturn(null); + return request; + } + } + + @BeforeEach + public void resetManager() { + reset(MANAGER); + } + + @Override + protected Application configure() { + try { + forceSet( + TestProperties.CONTAINER_PORT, String.valueOf(RESTUtils.findAvailablePort(2000, 3000))); + } catch (IOException e) { + throw new RuntimeException(e); + } + + ResourceConfig resourceConfig = new ResourceConfig(); + resourceConfig.register(TestableIdpUserOperations.class); + resourceConfig.register( + new AbstractBinder() { + @Override + protected void configure() { + bindFactory(MockServletRequestFactory.class).to(HttpServletRequest.class); + } + }); + + return resourceConfig; + } + + @Test + public void testAddUser() { + CreateUserRequest req = new CreateUserRequest("user1", "Passw0rd"); + IdpUserDTO user = buildUser("user1"); + + when(MANAGER.createUser("user1", "Passw0rd")).thenReturn(user); + Review Comment: These REST tests use passwords like `Passw0rd` / `Passw0rd1`, but the actual implementation enforces a 12–64 character password length. Using invalid passwords in the “success” path makes the tests diverge from real behavior and OpenAPI constraints. Please update the test inputs to use passwords that satisfy the real validation rules. ########## core/src/main/java/org/apache/gravitino/authorization/IdpUserManager.java: ########## @@ -0,0 +1,172 @@ +/* + * 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.gravitino.authorization; + +import com.google.common.base.Preconditions; +import java.util.List; +import java.util.Optional; +import org.apache.commons.lang3.StringUtils; +import org.apache.gravitino.Config; +import org.apache.gravitino.Configs; +import org.apache.gravitino.GravitinoEnv; +import org.apache.gravitino.dto.IdpUserDTO; +import org.apache.gravitino.exceptions.ForbiddenException; +import org.apache.gravitino.exceptions.NoSuchUserException; +import org.apache.gravitino.exceptions.UserAlreadyExistsException; +import org.apache.gravitino.idp.basic.password.PasswordHasher; +import org.apache.gravitino.idp.basic.password.PasswordHasherFactory; +import org.apache.gravitino.storage.IdGenerator; +import org.apache.gravitino.storage.relational.po.IdpUserPO; +import org.apache.gravitino.storage.relational.service.IdpUserMetaService; +import org.apache.gravitino.utils.PrincipalUtils; + +/** Manager for built-in IdP user management APIs. */ +public class IdpUserManager { + private static final long INITIAL_VERSION = 1L; + + private final Config config; + private final IdGenerator idGenerator; + private final IdpUserMetaService userMetaService; + private final PasswordHasher passwordHasher; + + public static IdpUserManager fromEnvironment() { + return fromEnvironment(PasswordHasherFactory.create()); + } + + public static IdpUserManager fromEnvironment(PasswordHasher passwordHasher) { + return new IdpUserManager( + GravitinoEnv.getInstance().config(), + GravitinoEnv.getInstance().idGenerator(), + IdpUserMetaService.getInstance(), + passwordHasher); + } + + IdpUserManager( + Config config, + IdGenerator idGenerator, + IdpUserMetaService userMetaService, + PasswordHasher passwordHasher) { + this.config = config; + this.idGenerator = idGenerator; + this.userMetaService = userMetaService; + this.passwordHasher = passwordHasher; + } + + public IdpUserDTO createUser(String userName, String password) { + ensureServiceAdmin(); + validateUserName(userName); + validatePassword(password); + if (userMetaService().findUser(userName).isPresent()) { + throw new UserAlreadyExistsException("Built-in IdP user %s already exists", userName); + } + + userMetaService() + .createUser( + IdpUserPO.builder() + .withUserId(nextId()) + .withUserName(userName) + .withPasswordHash(passwordHasher.hash(password)) + .withCurrentVersion(INITIAL_VERSION) + .withLastVersion(INITIAL_VERSION) + .withDeletedAt(0L) + .build()); + return getUser(userName); + } + + public IdpUserDTO getUser(String userName) { + validateUserName(userName); + IdpUserPO userPO = + userMetaService() + .findUser(userName) + .orElseThrow( + () -> new NoSuchUserException("Built-in IdP user %s does not exist", userName)); + return toUserDTO(userPO); + } Review Comment: `getUser` does not call `ensureServiceAdmin()`, so any (potentially unauthenticated/unauthorized) caller can retrieve built-in IdP user details (including group memberships) if they can hit the REST endpoint. For a management API this is a user-enumeration / information disclosure risk. Consider enforcing service-admin access here (or explicitly restricting reads to self-only if that’s the intended policy) and update REST/OpenAPI/tests accordingly. ########## server/src/test/java/org/apache/gravitino/server/web/rest/TestIdpUserOperations.java: ########## @@ -0,0 +1,396 @@ +/* + * 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.gravitino.server.web.rest; + +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import javax.servlet.http.HttpServletRequest; +import javax.ws.rs.client.Entity; +import javax.ws.rs.core.Application; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import org.apache.gravitino.authorization.IdpUserManager; +import org.apache.gravitino.dto.IdpUserDTO; +import org.apache.gravitino.dto.requests.CreateUserRequest; +import org.apache.gravitino.dto.requests.ResetPasswordRequest; +import org.apache.gravitino.dto.responses.ErrorConstants; +import org.apache.gravitino.dto.responses.ErrorResponse; +import org.apache.gravitino.dto.responses.IdpUserResponse; +import org.apache.gravitino.dto.responses.RemoveResponse; +import org.apache.gravitino.exceptions.ForbiddenException; +import org.apache.gravitino.exceptions.NoSuchUserException; +import org.apache.gravitino.exceptions.UserAlreadyExistsException; +import org.apache.gravitino.rest.RESTUtils; +import org.glassfish.hk2.utilities.binding.AbstractBinder; +import org.glassfish.jersey.server.ResourceConfig; +import org.glassfish.jersey.test.TestProperties; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class TestIdpUserOperations extends BaseOperationsTest { + + private static final IdpUserManager MANAGER = mock(IdpUserManager.class); + + public static class TestableIdpUserOperations extends IdpUserOperations { + public TestableIdpUserOperations() { + super(MANAGER); + } + } + + private static class MockServletRequestFactory extends ServletRequestFactoryBase { + @Override + public HttpServletRequest get() { + HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getRemoteUser()).thenReturn(null); + return request; + } + } + + @BeforeEach + public void resetManager() { + reset(MANAGER); + } + + @Override + protected Application configure() { + try { + forceSet( + TestProperties.CONTAINER_PORT, String.valueOf(RESTUtils.findAvailablePort(2000, 3000))); + } catch (IOException e) { + throw new RuntimeException(e); + } + + ResourceConfig resourceConfig = new ResourceConfig(); + resourceConfig.register(TestableIdpUserOperations.class); + resourceConfig.register( + new AbstractBinder() { + @Override + protected void configure() { + bindFactory(MockServletRequestFactory.class).to(HttpServletRequest.class); + } + }); + + return resourceConfig; + } + + @Test + public void testAddUser() { + CreateUserRequest req = new CreateUserRequest("user1", "Passw0rd"); + IdpUserDTO user = buildUser("user1"); + + when(MANAGER.createUser("user1", "Passw0rd")).thenReturn(user); + + // test with IllegalRequest + CreateUserRequest illegalReq = new CreateUserRequest("", "Passw0rd"); + Response illegalResp = + target("/idp/users") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(illegalReq, MediaType.APPLICATION_JSON_TYPE)); + Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), illegalResp.getStatus()); + Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE, illegalResp.getMediaType()); + + ErrorResponse illegalResponse = illegalResp.readEntity(ErrorResponse.class); + Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE, illegalResponse.getCode()); + Assertions.assertEquals( + IllegalArgumentException.class.getSimpleName(), illegalResponse.getType()); + + Response resp = + target("/idp/users") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(req, MediaType.APPLICATION_JSON_TYPE)); + + Assertions.assertEquals(Response.Status.OK.getStatusCode(), resp.getStatus()); + Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE, resp.getMediaType()); + + IdpUserResponse userResponse = resp.readEntity(IdpUserResponse.class); + Assertions.assertEquals(0, userResponse.getCode()); + Assertions.assertEquals("user1", userResponse.getUser().name()); + Assertions.assertNotNull(userResponse.getUser().groups()); + Assertions.assertTrue(userResponse.getUser().groups().isEmpty()); + + // Test to throw UserAlreadyExistsException + doThrow(new UserAlreadyExistsException("mock error")) + .when(MANAGER) + .createUser("user1", "Passw0rd"); + Response resp1 = + target("/idp/users") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(req, MediaType.APPLICATION_JSON_TYPE)); + + Assertions.assertEquals(Response.Status.CONFLICT.getStatusCode(), resp1.getStatus()); + Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE, resp1.getMediaType()); + + ErrorResponse errorResponse = resp1.readEntity(ErrorResponse.class); + Assertions.assertEquals(ErrorConstants.ALREADY_EXISTS_CODE, errorResponse.getCode()); + Assertions.assertEquals( + UserAlreadyExistsException.class.getSimpleName(), errorResponse.getType()); + + // Test to throw internal RuntimeException + reset(MANAGER); + when(MANAGER.createUser("user1", "Passw0rd")).thenThrow(new RuntimeException("mock error")); + Response resp2 = + target("/idp/users") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(req, MediaType.APPLICATION_JSON_TYPE)); + + Assertions.assertEquals( + Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp2.getStatus()); + + ErrorResponse errorResponse1 = resp2.readEntity(ErrorResponse.class); + Assertions.assertEquals(ErrorConstants.INTERNAL_ERROR_CODE, errorResponse1.getCode()); + Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResponse1.getType()); + Assertions.assertFalse(errorResponse1.getMessage().contains("under metalake")); + Assertions.assertTrue(errorResponse1.getMessage().contains("built-in IdP user")); + } + + @Test + public void testAddUserForbidden() { + CreateUserRequest req = new CreateUserRequest("user1", "Passw0rd"); + doThrow(new ForbiddenException("mock forbidden")).when(MANAGER).createUser("user1", "Passw0rd"); + + Response resp = + target("/idp/users") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(req, MediaType.APPLICATION_JSON_TYPE)); + + Assertions.assertEquals(Response.Status.FORBIDDEN.getStatusCode(), resp.getStatus()); + + ErrorResponse errorResponse = resp.readEntity(ErrorResponse.class); + Assertions.assertEquals(ErrorConstants.FORBIDDEN_CODE, errorResponse.getCode()); + Assertions.assertEquals(ForbiddenException.class.getSimpleName(), errorResponse.getType()); + } + + @Test + public void testAddUserWithNullRequest() { + Response resp = + target("/idp/users") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(null, MediaType.APPLICATION_JSON_TYPE)); + + Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), resp.getStatus()); + + ErrorResponse errorResponse = resp.readEntity(ErrorResponse.class); + Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE, errorResponse.getCode()); + Assertions.assertEquals( + IllegalArgumentException.class.getSimpleName(), errorResponse.getType()); + Assertions.assertTrue(errorResponse.getMessage().contains("Request body cannot be null")); + } + + @Test + public void testGetUser() { + IdpUserDTO user = buildUser("user1"); + + when(MANAGER.getUser("user1")).thenReturn(user); + + Response resp = + target("/idp/users/user1") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .get(); + + Assertions.assertEquals(Response.Status.OK.getStatusCode(), resp.getStatus()); + Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE, resp.getMediaType()); + + IdpUserResponse userResponse = resp.readEntity(IdpUserResponse.class); + Assertions.assertEquals(0, userResponse.getCode()); + Assertions.assertEquals("user1", userResponse.getUser().name()); + Assertions.assertNotNull(userResponse.getUser().groups()); + Assertions.assertTrue(userResponse.getUser().groups().isEmpty()); + + // Test to throw NoSuchUserException + doThrow(new NoSuchUserException("mock error")).when(MANAGER).getUser("user1"); + Response resp1 = + target("/idp/users/user1") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .get(); + + Assertions.assertEquals(Response.Status.NOT_FOUND.getStatusCode(), resp1.getStatus()); + Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE, resp1.getMediaType()); + + ErrorResponse errorResponse = resp1.readEntity(ErrorResponse.class); + Assertions.assertEquals(ErrorConstants.NOT_FOUND_CODE, errorResponse.getCode()); + Assertions.assertEquals(NoSuchUserException.class.getSimpleName(), errorResponse.getType()); + + // Test to throw internal RuntimeException + reset(MANAGER); + when(MANAGER.getUser("user1")).thenThrow(new RuntimeException("mock error")); + Response resp2 = + target("/idp/users/user1") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .get(); + + Assertions.assertEquals( + Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp2.getStatus()); + + ErrorResponse errorResponse1 = resp2.readEntity(ErrorResponse.class); + Assertions.assertEquals(ErrorConstants.INTERNAL_ERROR_CODE, errorResponse1.getCode()); + Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResponse1.getType()); + } + + @Test + public void testResetPassword() { + ResetPasswordRequest req = new ResetPasswordRequest("Passw0rd1"); + IdpUserDTO user = buildUser("user1"); + + when(MANAGER.resetPassword("user1", "Passw0rd1")).thenReturn(user); + Review Comment: These REST tests use passwords like `Passw0rd` / `Passw0rd1`, but the actual implementation enforces a 12–64 character password length. Using invalid passwords in the “success” path makes the tests diverge from real behavior and OpenAPI constraints. Please update the test inputs to use passwords that satisfy the real validation rules. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
