From a774a67ffa8c190e94d695ecf1c2a272d7dbd4f7 Mon Sep 17 00:00:00 2001 From: Sriharsha Chintalapani Date: Mon, 9 May 2022 23:59:04 -0700 Subject: [PATCH] Fix #4812: UserResources patch operation resetting inheritedRoles and AuthenticationMechanism (#4814) --- .../catalog/jdbi3/UserRepository.java | 8 ++++-- .../catalog/resources/teams/UserResource.java | 8 +++--- .../security/jwt/JWTTokenGenerator.java | 26 ++++++++++++------- .../schema/entity/teams/authN/jwtAuth.json | 4 +++ .../resources/teams/UserResourceTest.java | 9 +++---- .../security/JWTTokenGeneratorTest.java | 18 +++++++------ ingestion-core/src/metadata/_version.py | 2 +- 7 files changed, 45 insertions(+), 30 deletions(-) diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/UserRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/UserRepository.java index 7ed177515fa..d43257dcccb 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/UserRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/UserRepository.java @@ -44,7 +44,7 @@ import org.openmetadata.catalog.util.JsonUtils; @Slf4j public class UserRepository extends EntityRepository { - static final String USER_PATCH_FIELDS = "profile,roles,teams"; + static final String USER_PATCH_FIELDS = "profile,roles,teams,inheritedRoles,authenticationMechanism"; static final String USER_UPDATE_FIELDS = "profile,roles,teams"; public UserRepository(CollectionDAO dao) { @@ -78,7 +78,11 @@ public class UserRepository extends EntityRepository { @Override public void restorePatchAttributes(User original, User updated) { // Patch can't make changes to following fields. Ignore the changes - updated.withId(original.getId()).withName(original.getName()).withInheritedRoles(original.getInheritedRoles()); + updated + .withId(original.getId()) + .withName(original.getName()) + .withInheritedRoles(original.getInheritedRoles()) + .withAuthenticationMechanism(original.getAuthenticationMechanism()); } private List getTeamDefaultRoles(User user) throws IOException { diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/teams/UserResource.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/teams/UserResource.java index a4205d7a4b9..c0998868c4c 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/teams/UserResource.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/teams/UserResource.java @@ -67,6 +67,7 @@ import org.openmetadata.catalog.security.Authorizer; import org.openmetadata.catalog.security.SecurityUtil; import org.openmetadata.catalog.security.jwt.JWTTokenGenerator; import org.openmetadata.catalog.teams.authn.JWTAuthMechanism; +import org.openmetadata.catalog.teams.authn.JWTTokenExpiry; import org.openmetadata.catalog.type.EntityHistory; import org.openmetadata.catalog.type.Include; import org.openmetadata.catalog.util.EntityUtil.Fields; @@ -364,14 +365,14 @@ public class UserResource extends EntityResource { responseCode = "200", description = "The user ", content = - @Content(mediaType = "application/json", schema = @Schema(implementation = JWTAuthMechanism.class))), + @Content(mediaType = "application/json", schema = @Schema(implementation = JWTTokenExpiry.class))), @ApiResponse(responseCode = "400", description = "Bad request") }) public JWTAuthMechanism generateToken( @Context UriInfo uriInfo, @Context SecurityContext securityContext, @PathParam("id") String id, - JWTAuthMechanism jwtAuthMechanism) + JWTTokenExpiry jwtTokenExpiry) throws IOException { User user = dao.get(uriInfo, id, Fields.EMPTY_FIELDS); @@ -379,8 +380,7 @@ public class UserResource extends EntityResource { throw new IllegalArgumentException("Generating JWT token is only supported for bot users"); } SecurityUtil.authorizeAdmin(authorizer, securityContext, ADMIN); - String token = jwtTokenGenerator.generateJWTToken(user, jwtAuthMechanism.getJWTTokenExpiry()); - jwtAuthMechanism.setJWTToken(token); + JWTAuthMechanism jwtAuthMechanism = jwtTokenGenerator.generateJWTToken(user, jwtTokenExpiry); AuthenticationMechanism authenticationMechanism = new AuthenticationMechanism().withConfig(jwtAuthMechanism).withAuthType(AuthenticationMechanism.AuthType.JWT); user.setAuthenticationMechanism(authenticationMechanism); diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/jwt/JWTTokenGenerator.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/jwt/JWTTokenGenerator.java index ad9115189d6..aea08fd9361 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/jwt/JWTTokenGenerator.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/jwt/JWTTokenGenerator.java @@ -18,6 +18,7 @@ import java.util.List; import lombok.Getter; import lombok.extern.slf4j.Slf4j; import org.openmetadata.catalog.entity.teams.User; +import org.openmetadata.catalog.teams.authn.JWTAuthMechanism; import org.openmetadata.catalog.teams.authn.JWTTokenExpiry; @Slf4j @@ -67,19 +68,24 @@ public class JWTTokenGenerator { } } - public String generateJWTToken(User user, JWTTokenExpiry expiry) { + public JWTAuthMechanism generateJWTToken(User user, JWTTokenExpiry expiry) { try { + JWTAuthMechanism jwtAuthMechanism = new JWTAuthMechanism().withJWTTokenExpiry(expiry); Algorithm algorithm = Algorithm.RSA256(null, privateKey); Date expires = getExpiryDate(expiry); - return JWT.create() - .withIssuer(issuer) - .withKeyId(kid) - .withClaim("sub", user.getName()) - .withClaim("email", user.getEmail()) - .withClaim("isBot", true) - .withIssuedAt(new Date(System.currentTimeMillis())) - .withExpiresAt(expires) - .sign(algorithm); + String token = + JWT.create() + .withIssuer(issuer) + .withKeyId(kid) + .withClaim("sub", user.getName()) + .withClaim("email", user.getEmail()) + .withClaim("isBot", true) + .withIssuedAt(new Date(System.currentTimeMillis())) + .withExpiresAt(expires) + .sign(algorithm); + jwtAuthMechanism.setJWTToken(token); + jwtAuthMechanism.setJWTTokenExpiresAt(expires != null ? expires.getTime() : null); + return jwtAuthMechanism; } catch (Exception e) { throw new JWTCreationException("Failed to generate JWT Token. Please check your OpenMetadata Configuration.", e); } diff --git a/catalog-rest-service/src/main/resources/json/schema/entity/teams/authN/jwtAuth.json b/catalog-rest-service/src/main/resources/json/schema/entity/teams/authN/jwtAuth.json index 0ba8f2637f9..9f8ea2618d9 100644 --- a/catalog-rest-service/src/main/resources/json/schema/entity/teams/authN/jwtAuth.json +++ b/catalog-rest-service/src/main/resources/json/schema/entity/teams/authN/jwtAuth.json @@ -32,6 +32,10 @@ "name": "Unlimited" } ] + }, + "JWTTokenExpiresAt": { + "description": "JWT Auth Token expiration time.", + "$ref": "../../../type/basic.json#/definitions/timestamp" } }, "additionalProperties": false, diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/UserResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/UserResourceTest.java index 94183c2b18a..a5ccc504faf 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/UserResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/UserResourceTest.java @@ -63,7 +63,6 @@ import java.util.Map; import java.util.TimeZone; import java.util.UUID; import java.util.function.Predicate; -import javax.ws.rs.client.WebTarget; import lombok.SneakyThrows; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; @@ -88,7 +87,6 @@ import org.openmetadata.catalog.resources.databases.TableResourceTest; import org.openmetadata.catalog.resources.locations.LocationResourceTest; import org.openmetadata.catalog.resources.teams.UserResource.UserList; import org.openmetadata.catalog.security.AuthenticationException; -import org.openmetadata.catalog.security.jwt.JWKSResponse; import org.openmetadata.catalog.teams.authn.JWTAuthMechanism; import org.openmetadata.catalog.teams.authn.JWTTokenExpiry; import org.openmetadata.catalog.type.ChangeDescription; @@ -696,7 +694,10 @@ public class UserResourceTest extends EntityResourceTest { authHeaders("ingestion-bot-jwt@email.com")); JWTAuthMechanism authMechanism = new JWTAuthMechanism().withJWTTokenExpiry(JWTTokenExpiry.Seven); TestUtils.put( - getResource(String.format("users/generateToken/%s", user.getId())), authMechanism, OK, ADMIN_AUTH_HEADERS); + getResource(String.format("users/generateToken/%s", user.getId())), + JWTTokenExpiry.Seven, + OK, + ADMIN_AUTH_HEADERS); user = getEntity(user.getId(), ADMIN_AUTH_HEADERS); assertNull(user.getAuthenticationMechanism()); JWTAuthMechanism jwtAuthMechanism = @@ -717,8 +718,6 @@ public class UserResourceTest extends EntityResourceTest { } private DecodedJWT decodedJWT(String token) throws MalformedURLException, JwkException, HttpResponseException { - WebTarget target = getConfigResource("jwks"); - JWKSResponse auth = TestUtils.get(target, JWKSResponse.class, TEST_AUTH_HEADERS); DecodedJWT jwt; try { jwt = JWT.decode(token); diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/security/JWTTokenGeneratorTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/security/JWTTokenGeneratorTest.java index c81ec1e61eb..d669a295a0e 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/security/JWTTokenGeneratorTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/security/JWTTokenGeneratorTest.java @@ -22,6 +22,7 @@ import org.junit.jupiter.api.TestInstance; import org.openmetadata.catalog.entity.teams.User; import org.openmetadata.catalog.security.jwt.JWTTokenConfiguration; import org.openmetadata.catalog.security.jwt.JWTTokenGenerator; +import org.openmetadata.catalog.teams.authn.JWTAuthMechanism; import org.openmetadata.catalog.teams.authn.JWTTokenExpiry; @Slf4j @@ -44,29 +45,30 @@ public class JWTTokenGeneratorTest { } @Test - void testGenerateJWTToken() throws NoSuchAlgorithmException, InvalidKeySpecException, IOException { + void testGenerateJWTToken() { User user = new User() .withEmail("ingestion-bot@open-metadata.org") .withName("ingestion-bot") .withDisplayName("ingestion-bot"); - String token = jwtTokenGenerator.generateJWTToken(user, JWTTokenExpiry.Seven); - DecodedJWT jwt = decodedJWT(token); + JWTAuthMechanism jwtAuthMechanism = jwtTokenGenerator.generateJWTToken(user, JWTTokenExpiry.Seven); + DecodedJWT jwt = decodedJWT(jwtAuthMechanism.getJWTToken()); assertEquals(jwt.getClaims().get("sub").asString(), "ingestion-bot"); Date date = jwt.getExpiresAt(); long daysBetween = ((date.getTime() - jwt.getIssuedAt().getTime()) / (1000 * 60 * 60 * 24)); assertTrue(daysBetween >= 6); - token = jwtTokenGenerator.generateJWTToken(user, JWTTokenExpiry.Ninety); - jwt = decodedJWT(token); + jwtAuthMechanism = jwtTokenGenerator.generateJWTToken(user, JWTTokenExpiry.Ninety); + jwt = decodedJWT(jwtAuthMechanism.getJWTToken()); date = jwt.getExpiresAt(); daysBetween = ((date.getTime() - jwt.getIssuedAt().getTime()) / (1000 * 60 * 60 * 24)); assertTrue(daysBetween >= 89); - token = jwtTokenGenerator.generateJWTToken(user, JWTTokenExpiry.Unlimited); - jwt = decodedJWT(token); + jwtAuthMechanism = jwtTokenGenerator.generateJWTToken(user, JWTTokenExpiry.Unlimited); + jwt = decodedJWT(jwtAuthMechanism.getJWTToken()); assertNull(jwt.getExpiresAt()); + assertNull(jwtAuthMechanism.getJWTTokenExpiresAt()); } - private DecodedJWT decodedJWT(String token) throws IOException, NoSuchAlgorithmException, InvalidKeySpecException { + private DecodedJWT decodedJWT(String token) { RSAPublicKey publicKey = jwtTokenGenerator.getPublicKey(); Algorithm algorithm = Algorithm.RSA256(publicKey, null); JWTVerifier verifier = JWT.require(algorithm).withIssuer(jwtTokenConfiguration.getJWTIssuer()).build(); diff --git a/ingestion-core/src/metadata/_version.py b/ingestion-core/src/metadata/_version.py index a4b00be2148..1b48c1567f5 100644 --- a/ingestion-core/src/metadata/_version.py +++ b/ingestion-core/src/metadata/_version.py @@ -7,5 +7,5 @@ Provides metadata version information. from incremental import Version -__version__ = Version("metadata", 0, 11, 0, dev=2) +__version__ = Version("metadata", 0, 11, 0, dev=3) __all__ = ["__version__"]