From 145befd172c8541d0a81b1efda5774c29dfba977 Mon Sep 17 00:00:00 2001 From: amvanbaren Date: Mon, 13 May 2024 22:03:37 +0300 Subject: [PATCH] Simplify publisher agreement --- .../java/org/eclipse/openvsx/UserAPI.java | 10 +- .../eclipse/openvsx/admin/AdminService.java | 11 +- .../openvsx/eclipse/EclipseService.java | 251 +++++------------- .../openvsx/eclipse/PublisherAgreement.java | 42 +++ .../eclipse/PublisherAgreementResponse.java | 22 -- .../eclipse/PublisherComplianceChecker.java | 22 +- .../eclipse/openvsx/entities/EclipseData.java | 79 ------ .../entities/EclipseDataConverter.java | 49 ---- .../eclipse/openvsx/entities/UserData.java | 16 +- .../openvsx/security/OAuth2UserServices.java | 4 +- .../V1_44__UserData_Add_EclipsePersonId.sql | 3 + .../openvsx/eclipse/EclipseServiceTest.java | 79 ++---- 12 files changed, 164 insertions(+), 424 deletions(-) create mode 100644 server/src/main/java/org/eclipse/openvsx/eclipse/PublisherAgreement.java delete mode 100644 server/src/main/java/org/eclipse/openvsx/entities/EclipseData.java delete mode 100644 server/src/main/java/org/eclipse/openvsx/entities/EclipseDataConverter.java create mode 100644 server/src/main/resources/db/migration/V1_44__UserData_Add_EclipsePersonId.sql diff --git a/server/src/main/java/org/eclipse/openvsx/UserAPI.java b/server/src/main/java/org/eclipse/openvsx/UserAPI.java index 4df4b5105..d4fb2e228 100644 --- a/server/src/main/java/org/eclipse/openvsx/UserAPI.java +++ b/server/src/main/java/org/eclipse/openvsx/UserAPI.java @@ -20,6 +20,8 @@ import org.eclipse.openvsx.util.ErrorResultException; import org.eclipse.openvsx.util.NotFoundException; import org.eclipse.openvsx.util.UrlUtil; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.http.CacheControl; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; @@ -44,6 +46,8 @@ public class UserAPI { private final static int TOKEN_DESCRIPTION_SIZE = 255; + protected final Logger logger = LoggerFactory.getLogger(UserAPI.class); + private final RepositoryService repositories; private final UserService users; private final EclipseService eclipse; @@ -108,7 +112,11 @@ public UserJson getUserData() { json.role = user.getRole(); json.tokensUrl = createApiUrl(serverUrl, "user", "tokens"); json.createTokenUrl = createApiUrl(serverUrl, "user", "token", "create"); - eclipse.enrichUserJson(json, user); + try { + eclipse.enrichUserJson(json, user); + } catch (ErrorResultException e) { + logger.error("Failed to enrich UserJson", e); + } return json; } diff --git a/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java b/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java index 348629a31..c6a4d5a56 100644 --- a/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java +++ b/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java @@ -265,7 +265,12 @@ public UserPublishInfoJson getUserPublishInfo(String provider, String loginName) var userPublishInfo = new UserPublishInfoJson(); userPublishInfo.user = user.toUserJson(); - eclipse.enrichUserJson(userPublishInfo.user, user); + try { + eclipse.enrichUserJson(userPublishInfo.user, user); + } catch (ErrorResultException e) { + userPublishInfo.user.error = e.getMessage(); + } + userPublishInfo.activeAccessTokenNum = (int) repositories.countActiveAccessTokens(user); var extVersions = repositories.findLatestVersions(user); var types = new String[]{DOWNLOAD, MANIFEST, ICON, README, LICENSE, CHANGELOG, VSIXMANIFEST}; @@ -296,9 +301,7 @@ public ResultJson revokePublisherContributions(String provider, String loginName } // Send a DELETE request to the Eclipse publisher agreement API - if (eclipse.isActive() && user.getEclipseData() != null - && user.getEclipseData().publisherAgreement != null - && user.getEclipseData().publisherAgreement.isActive) { + if (eclipse.isActive() && user.getEclipsePersonId() != null) { eclipse.revokePublisherAgreement(user, admin); } diff --git a/server/src/main/java/org/eclipse/openvsx/eclipse/EclipseService.java b/server/src/main/java/org/eclipse/openvsx/eclipse/EclipseService.java index 3a55a5da1..ad4dfec1d 100644 --- a/server/src/main/java/org/eclipse/openvsx/eclipse/EclipseService.java +++ b/server/src/main/java/org/eclipse/openvsx/eclipse/EclipseService.java @@ -21,7 +21,6 @@ import org.apache.commons.lang3.StringUtils; import org.eclipse.openvsx.ExtensionService; import org.eclipse.openvsx.entities.AuthToken; -import org.eclipse.openvsx.entities.EclipseData; import org.eclipse.openvsx.entities.UserData; import org.eclipse.openvsx.json.UserJson; import org.eclipse.openvsx.security.TokenService; @@ -33,8 +32,6 @@ import org.springframework.beans.factory.annotation.Value; import org.springframework.http.*; import org.springframework.stereotype.Component; -import org.springframework.transaction.TransactionException; -import org.springframework.transaction.support.TransactionTemplate; import org.springframework.web.client.HttpStatusCodeException; import org.springframework.web.client.RestClientException; import org.springframework.web.client.RestTemplate; @@ -48,8 +45,6 @@ import java.util.Arrays; import java.util.List; import java.util.Map; -import java.util.function.Consumer; -import java.util.function.Function; import java.util.regex.Pattern; @Component @@ -62,14 +57,13 @@ public class EclipseService { .append(DateTimeFormatter.ISO_LOCAL_TIME) .toFormatter(); - private static final TypeReference> TYPE_LIST_STRING = new TypeReference>() {}; - private static final TypeReference> TYPE_LIST_PROFILE = new TypeReference>() {}; - private static final TypeReference> TYPE_LIST_AGREEMENT = new TypeReference>() {}; + private static final TypeReference> TYPE_LIST_STRING = new TypeReference<>() {}; + private static final TypeReference> TYPE_LIST_PROFILE = new TypeReference<>() {}; + private static final TypeReference> TYPE_LIST_AGREEMENT = new TypeReference<>() {}; protected final Logger logger = LoggerFactory.getLogger(EclipseService.class); private final TokenService tokens; - private final TransactionTemplate transactions; private final ExtensionService extensions; private final EntityManager entityManager; private final RestTemplate restTemplate; @@ -82,19 +76,14 @@ public class EclipseService { @Value("${ovsx.eclipse.publisher-agreement.version:}") String publisherAgreementVersion; - @Value("${ovsx.eclipse.publisher-agreement.timezone:}") - String publisherAgreementTimeZone; - public EclipseService( TokenService tokens, - TransactionTemplate transactions, ExtensionService extensions, EntityManager entityManager, RestTemplate restTemplate, ObservationRegistry observations ) { this.tokens = tokens; - this.transactions = transactions; this.extensions = extensions; this.entityManager = entityManager; this.restTemplate = restTemplate; @@ -103,19 +92,6 @@ public EclipseService( this.observations = observations; } - private final Function parseDate = dateString -> { - try { - var local = LocalDateTime.parse(dateString, CUSTOM_DATE_TIME); - if (StringUtils.isEmpty(publisherAgreementTimeZone)) { - return local; - } - return TimeUtil.convertToUTC(local, publisherAgreementTimeZone); - } catch (DateTimeParseException exc) { - logger.error("Failed to parse timestamp.", exc); - return null; - } - }; - public boolean isActive() { return !StringUtils.isEmpty(publisherAgreementVersion); } @@ -134,18 +110,17 @@ public void checkPublisherAgreement(UserData user) { if (user.getProvider() == null) { return; } - var eclipseData = user.getEclipseData(); - if (eclipseData == null || eclipseData.personId == null) { + var personId = user.getEclipsePersonId(); + if (personId == null) { throw new ErrorResultException("You must log in with an Eclipse Foundation account and sign a Publisher Agreement before publishing any extension."); } - var profile = getPublicProfile(eclipseData.personId); - if (profile.publisherAgreements == null || profile.publisherAgreements.openVsx == null - || profile.publisherAgreements.openVsx.version == null) { + var agreement = getPublisherAgreement(user); + if (agreement == null || agreement.version == null) { throw new ErrorResultException("You must sign a Publisher Agreement with the Eclipse Foundation before publishing any extension."); } - if (!publisherAgreementVersion.equals(profile.publisherAgreements.openVsx.version)) { + if (!publisherAgreementVersion.equals(agreement.version)) { throw new ErrorResultException("Your Publisher Agreement with the Eclipse Foundation is outdated (version " - + profile.publisherAgreements.openVsx.version + "). The current version is " + + agreement.version + "). The current version is " + publisherAgreementVersion + "."); } }); @@ -155,35 +130,9 @@ public void checkPublisherAgreement(UserData user) { * Update the given user data with a profile obtained from Eclipse API. */ @Transactional - public EclipseData updateUserData(UserData user, EclipseProfile profile) { - EclipseData eclipseData; - if (user.getEclipseData() == null) { - eclipseData = new EclipseData(); - } else { - // We need to clone and reset the data to ensure that Hibernate will persist the updated state - eclipseData = user.getEclipseData().clone(); - } - - if (StringUtils.isEmpty(eclipseData.personId)) { - eclipseData.personId = profile.name; - } - if (profile.publisherAgreements != null) { - if (profile.publisherAgreements.openVsx == null) { - if (eclipseData.publisherAgreement != null) { - eclipseData.publisherAgreement.isActive = false; - } - } else if (!StringUtils.isEmpty(profile.publisherAgreements.openVsx.version)) { - if (eclipseData.publisherAgreement == null) { - eclipseData.publisherAgreement = new EclipseData.PublisherAgreement(); - } - eclipseData.publisherAgreement.isActive = true; - eclipseData.publisherAgreement.version = profile.publisherAgreements.openVsx.version; - } - } - - user.setEclipseData(eclipseData); - entityManager.merge(user); - return eclipseData; + public void updateUserData(UserData user, EclipseProfile profile) { + user = entityManager.merge(user); + user.setEclipsePersonId(profile.name); } /** @@ -195,74 +144,34 @@ public void enrichUserJson(UserJson json, UserData user) { } json.publisherAgreement = new UserJson.PublisherAgreement(); - var eclipseData = user.getEclipseData(); - if (eclipseData == null) { + var personId = user.getEclipsePersonId(); + if (personId == null) { json.publisherAgreement.status = "none"; return; } - // Update the internal data from the Eclipse profile - if (eclipseData.personId != null) { - try { - var profile = getPublicProfile(eclipseData.personId); - eclipseData = transactions.execute(status -> updateUserData(user, profile)); - } catch (ErrorResultException | TransactionException exc) { - // Continue with the information that is currently in the DB - } - } - - // Add information on the publisher agreement status - var agreement = eclipseData.publisherAgreement; - if (agreement == null || !agreement.isActive || agreement.version == null) - json.publisherAgreement.status = "none"; - else if (publisherAgreementVersion.equals(agreement.version)) - json.publisherAgreement.status = "signed"; - else - json.publisherAgreement.status = "outdated"; - if (agreement != null && agreement.timestamp != null) - json.publisherAgreement.timestamp = TimeUtil.toUTCString(agreement.timestamp); - // Report user as logged in only if there is a usabe token: // we need the token to access the Eclipse REST API - if (eclipseData.personId != null && tokens.isUsable(user.getEclipseToken())) { + if (tokens.isUsable(user.getEclipseToken())) { var eclipseLogin = new UserJson(); eclipseLogin.provider = "eclipse"; - eclipseLogin.loginName = eclipseData.personId; + eclipseLogin.loginName = personId; if (json.additionalLogins == null) json.additionalLogins = Lists.newArrayList(eclipseLogin); else json.additionalLogins.add(eclipseLogin); } - } - /** - * Get the publicly available user profile. - */ - public EclipseProfile getPublicProfile(String personId) { - return Observation.createNotStarted("EclipseService#getPublicProfile", observations).observe(() -> { - checkApiUrl(); - var urlTemplate = eclipseApiUrl + "account/profile/{personId}"; - var uriVariables = Map.of("personId", personId); - var headers = new HttpHeaders(); - headers.setAccept(Arrays.asList(MediaType.APPLICATION_JSON)); - var request = new HttpEntity(headers); - - try { - var response = restTemplate.exchange(urlTemplate, HttpMethod.GET, request, String.class, uriVariables); - return parseEclipseProfile(response); - } catch (RestClientException exc) { - if (exc instanceof HttpStatusCodeException) { - var status = ((HttpStatusCodeException) exc).getStatusCode(); - if (status == HttpStatus.NOT_FOUND) - throw new ErrorResultException("No Eclipse profile data available for user: " + personId); - } - - var url = UriComponentsBuilder.fromUriString(urlTemplate).build(uriVariables); - logger.error("Get request failed with URL: " + url, exc); - throw new ErrorResultException("Request for retrieving user profile failed: " + exc.getMessage(), - HttpStatus.INTERNAL_SERVER_ERROR); - } - }); + // Add information on the publisher agreement + var agreement = getPublisherAgreement(user); + if (agreement == null || !agreement.isActive || agreement.version == null) + json.publisherAgreement.status = "none"; + else if (publisherAgreementVersion.equals(agreement.version)) + json.publisherAgreement.status = "signed"; + else + json.publisherAgreement.status = "outdated"; + if (agreement != null && agreement.timestamp != null) + json.publisherAgreement.timestamp = TimeUtil.toUTCString(agreement.timestamp); } /** @@ -313,37 +222,23 @@ private EclipseProfile parseEclipseProfile(ResponseEntity response) { /** * Get the publisher agreement of the given user with the user's current access token. */ - public EclipseData.PublisherAgreement getPublisherAgreement(UserData user) { + public PublisherAgreement getPublisherAgreement(UserData user) { var eclipseToken = checkEclipseToken(user); - return getPublisherAgreement(user, eclipseToken.accessToken); - } - - /** - * Get the publisher agreement of the given user with the given access token. - */ - public EclipseData.PublisherAgreement getPublisherAgreement(UserData user, String accessToken) { - var eclipseData = user.getEclipseData(); - if (eclipseData == null || StringUtils.isEmpty(eclipseData.personId)) { + var personId = user.getEclipsePersonId(); + if (StringUtils.isEmpty(personId)) { return null; } checkApiUrl(); var urlTemplate = eclipseApiUrl + "openvsx/publisher_agreement/{personId}"; - var uriVariables = Map.of("personId", eclipseData.personId); + var uriVariables = Map.of("personId", personId); var headers = new HttpHeaders(); - headers.setBearerAuth(accessToken); + headers.setBearerAuth(eclipseToken.accessToken); headers.setAccept(Arrays.asList(MediaType.APPLICATION_JSON)); var request = new HttpEntity<>(headers); try { var json = restTemplate.exchange(urlTemplate, HttpMethod.GET, request, String.class, uriVariables); - var response = parseAgreementResponse(json); - - return updateEclipseData(user, ed -> { - ed.publisherAgreement = response.createEntityData(parseDate); - return ed.publisherAgreement; - }, ed -> { - ed.personId = response.personID; - }); + return parseAgreementResponse(json); } catch (RestClientException exc) { if (exc instanceof HttpStatusCodeException) { var status = ((HttpStatusCodeException) exc).getStatusCode(); @@ -364,7 +259,7 @@ public EclipseData.PublisherAgreement getPublisherAgreement(UserData user, Strin /** * Sign the publisher agreement on behalf of the given user. */ - public EclipseData.PublisherAgreement signPublisherAgreement(UserData user) { + public PublisherAgreement signPublisherAgreement(UserData user) { checkApiUrl(); var eclipseToken = checkEclipseToken(user); var headers = new HttpHeaders(); @@ -382,15 +277,7 @@ public EclipseData.PublisherAgreement signPublisherAgreement(UserData user) { extensions.reactivateExtensions(user); // Parse the response and store the publisher agreement metadata - var response = parseAgreementResponse(json); - var result = updateEclipseData(user, ed -> { - ed.publisherAgreement = response.createEntityData(parseDate); - return ed.publisherAgreement; - }, ed -> { - ed.personId = response.personID; - }); - return result; - + return parseAgreementResponse(json); } catch (RestClientException exc) { String message = exc.getMessage(); var statusCode = HttpStatus.INTERNAL_SERVER_ERROR; @@ -422,9 +309,10 @@ public EclipseData.PublisherAgreement signPublisherAgreement(UserData user) { } } - private PublisherAgreementResponse parseAgreementResponse(ResponseEntity response) { + private PublisherAgreement parseAgreementResponse(ResponseEntity response) { var json = response.getBody(); try { + PublisherAgreementResponse agreementResponse; if (json.startsWith("[\"")) { var error = objectMapper.readValue(json, TYPE_LIST_STRING); logger.error("Publisher agreement request failed:\n" + json); @@ -435,10 +323,17 @@ private PublisherAgreementResponse parseAgreementResponse(ResponseEntity if (profileList.isEmpty()) { throw new ErrorResultException("No publisher agreement available.", HttpStatus.INTERNAL_SERVER_ERROR); } - return profileList.get(0); + agreementResponse = profileList.get(0); } else { - return objectMapper.readValue(json, PublisherAgreementResponse.class); + agreementResponse = objectMapper.readValue(json, PublisherAgreementResponse.class); } + + var agreement = new PublisherAgreement(); + agreement.documentId = agreementResponse.documentID; + agreement.version = agreementResponse.version; + agreement.timestamp = parseDate(agreementResponse.effectiveDate); + agreement.isActive = TimeUtil.getCurrentUTC().isAfter(agreement.timestamp); + return agreement; } catch (JsonProcessingException exc) { logger.error("Failed to parse JSON response (" + response.getStatusCode() + "):\n" + json, exc); throw new ErrorResultException("Parsing publisher agreement response failed: " + exc.getMessage(), @@ -446,6 +341,15 @@ private PublisherAgreementResponse parseAgreementResponse(ResponseEntity } } + private LocalDateTime parseDate(String dateString) { + try { + return LocalDateTime.parse(dateString, CUSTOM_DATE_TIME); + } catch (DateTimeParseException exc) { + logger.error("Failed to parse timestamp.", exc); + return null; + } + } + /** * Revoke the given user's publisher agreement. If an admin user is given, * the admin's access token is used for the Eclipse API request, otherwise @@ -453,28 +357,18 @@ private PublisherAgreementResponse parseAgreementResponse(ResponseEntity */ public void revokePublisherAgreement(UserData user, UserData admin) { checkApiUrl(); - AuthToken eclipseToken; - if (admin == null) - eclipseToken = checkEclipseToken(user); - else - eclipseToken = checkEclipseToken(admin); + checkEclipseData(user); + + var eclipseToken = admin == null ? checkEclipseToken(user) : checkEclipseToken(admin); var headers = new HttpHeaders(); headers.setBearerAuth(eclipseToken.accessToken); var request = new HttpEntity<>(headers); - var eclipseData = checkEclipseData(user); var urlTemplate = eclipseApiUrl + "openvsx/publisher_agreement/{personId}"; - var uriVariables = Map.of("personId", eclipseData.personId); + var uriVariables = Map.of("personId", user.getEclipsePersonId()); try { var requestCallback = restTemplate.httpEntityCallback(request); restTemplate.execute(urlTemplate, HttpMethod.DELETE, requestCallback, null, uriVariables); - - if (eclipseData.publisherAgreement != null) { - updateEclipseData(user, ed -> { - ed.publisherAgreement.isActive = false; - return null; - }, NOP_INIT); - } } catch (RestClientException exc) { var url = UriComponentsBuilder.fromUriString(urlTemplate).build(uriVariables); logger.error("Delete request failed with URL: " + url, exc); @@ -497,35 +391,10 @@ private AuthToken checkEclipseToken(UserData user) { return eclipseToken; } - private EclipseData checkEclipseData(UserData user) { - var eclipseData = user.getEclipseData(); - if (eclipseData == null || StringUtils.isEmpty(eclipseData.personId)) { + private void checkEclipseData(UserData user) { + if (StringUtils.isEmpty(user.getEclipsePersonId())) { throw new ErrorResultException("Eclipse person ID is unavailable for user: " + user.getProvider() + "/" + user.getLoginName()); } - return eclipseData; } - - private static final Consumer NOP_INIT = ed -> {}; - - /** - * Update the Eclipse data of the given user and commit the change to the database. - */ - protected T updateEclipseData(UserData user, Function update, Consumer initialize) { - return transactions.execute(status -> { - EclipseData eclipseData; - if (user.getEclipseData() == null) { - eclipseData = new EclipseData(); - initialize.accept(eclipseData); - } else { - // We need to clone and reset the data to ensure that Hibernate will persist the updated state - eclipseData = user.getEclipseData().clone(); - } - var result = update.apply(eclipseData); - user.setEclipseData(eclipseData); - entityManager.merge(user); - return result; - }); - } - } \ No newline at end of file diff --git a/server/src/main/java/org/eclipse/openvsx/eclipse/PublisherAgreement.java b/server/src/main/java/org/eclipse/openvsx/eclipse/PublisherAgreement.java new file mode 100644 index 000000000..7ec4f2c74 --- /dev/null +++ b/server/src/main/java/org/eclipse/openvsx/eclipse/PublisherAgreement.java @@ -0,0 +1,42 @@ +/******************************************************************************** + * Copyright (c) 2020 TypeFox and others + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ +package org.eclipse.openvsx.eclipse; + +import java.time.LocalDateTime; +import java.util.Objects; + +public class PublisherAgreement { + + public boolean isActive; + + public String documentId; + + /** Version of the last signed publisher agreement. */ + public String version; + + /** Timestamp of the last signed publisher agreement. */ + public LocalDateTime timestamp; + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + PublisherAgreement that = (PublisherAgreement) o; + return isActive == that.isActive + && Objects.equals(documentId, that.documentId) + && Objects.equals(version, that.version) + && Objects.equals(timestamp, that.timestamp); + } + + @Override + public int hashCode() { + return Objects.hash(isActive, documentId, version, timestamp); + } +} diff --git a/server/src/main/java/org/eclipse/openvsx/eclipse/PublisherAgreementResponse.java b/server/src/main/java/org/eclipse/openvsx/eclipse/PublisherAgreementResponse.java index ce32312ab..876f2023b 100644 --- a/server/src/main/java/org/eclipse/openvsx/eclipse/PublisherAgreementResponse.java +++ b/server/src/main/java/org/eclipse/openvsx/eclipse/PublisherAgreementResponse.java @@ -9,27 +9,13 @@ ********************************************************************************/ package org.eclipse.openvsx.eclipse; -import java.time.LocalDateTime; -import java.util.function.Function; - import com.fasterxml.jackson.annotation.JsonProperty; -import org.eclipse.openvsx.entities.EclipseData; - /** * https://eclipsefdn.github.io/openvsx-publisher-agreement-specs/#/paths/~1publisher_agreement/post */ public class PublisherAgreementResponse { - public EclipseData.PublisherAgreement createEntityData(Function parseDate) { - var pub = new EclipseData.PublisherAgreement(); - pub.isActive = true; - pub.documentId = documentID; - pub.version = version; - pub.timestamp = parseDate.apply(effectiveDate); - return pub; - } - /** Unique identifier for an addressable object in the API. */ @JsonProperty("PersonID") String personID; @@ -50,17 +36,10 @@ public EclipseData.PublisherAgreement createEntityData(Function accessTokens) { diff --git a/server/src/main/java/org/eclipse/openvsx/entities/EclipseData.java b/server/src/main/java/org/eclipse/openvsx/entities/EclipseData.java deleted file mode 100644 index f1b5bf33c..000000000 --- a/server/src/main/java/org/eclipse/openvsx/entities/EclipseData.java +++ /dev/null @@ -1,79 +0,0 @@ -/******************************************************************************** - * Copyright (c) 2020 TypeFox and others - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Public License v. 2.0 which is available at - * http://www.eclipse.org/legal/epl-2.0. - * - * SPDX-License-Identifier: EPL-2.0 - ********************************************************************************/ -package org.eclipse.openvsx.entities; - -import java.io.Serializable; -import java.time.LocalDateTime; -import java.util.Objects; - -/** - * Additional information about Eclipse OAuth2 login and publisher agreement - * status attached to {@link UserData}. - * - *

This class is not mapped to a database entity, but parsed / serialized to - * JSON via a column converter. - */ -public class EclipseData implements Cloneable, Serializable { - - public String personId; - - public PublisherAgreement publisherAgreement; - - public static class PublisherAgreement implements Serializable { - - public boolean isActive; - - public String documentId; - - /** Version of the last signed publisher agreement. */ - public String version; - - /** Timestamp of the last signed publisher agreement. */ - public LocalDateTime timestamp; - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - PublisherAgreement that = (PublisherAgreement) o; - return isActive == that.isActive - && Objects.equals(documentId, that.documentId) - && Objects.equals(version, that.version) - && Objects.equals(timestamp, that.timestamp); - } - - @Override - public int hashCode() { - return Objects.hash(isActive, documentId, version, timestamp); - } - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - EclipseData that = (EclipseData) o; - return Objects.equals(personId, that.personId) && Objects.equals(publisherAgreement, that.publisherAgreement); - } - - @Override - public int hashCode() { - return Objects.hash(personId, publisherAgreement); - } - - @Override - public EclipseData clone() { - try { - return (EclipseData) super.clone(); - } catch (CloneNotSupportedException exc) { - throw new RuntimeException(exc); - } - } -} \ No newline at end of file diff --git a/server/src/main/java/org/eclipse/openvsx/entities/EclipseDataConverter.java b/server/src/main/java/org/eclipse/openvsx/entities/EclipseDataConverter.java deleted file mode 100644 index f011dab4b..000000000 --- a/server/src/main/java/org/eclipse/openvsx/entities/EclipseDataConverter.java +++ /dev/null @@ -1,49 +0,0 @@ -/******************************************************************************** - * Copyright (c) 2020 TypeFox and others - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Public License v. 2.0 which is available at - * http://www.eclipse.org/legal/epl-2.0. - * - * SPDX-License-Identifier: EPL-2.0 - ********************************************************************************/ -package org.eclipse.openvsx.entities; - -import jakarta.persistence.AttributeConverter; -import jakarta.persistence.Converter; - -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.ObjectMapper; - -@Converter -public class EclipseDataConverter implements AttributeConverter { - - private final ObjectMapper objectMapper = new ObjectMapper(); - - public EclipseDataConverter() { - objectMapper.registerModule(new com.fasterxml.jackson.datatype.jsr310.JavaTimeModule()); - } - - @Override - public String convertToDatabaseColumn(EclipseData data) { - if (data == null) - return null; - try { - return objectMapper.writeValueAsString(data); - } catch (JsonProcessingException exc) { - throw new RuntimeException("Failed to serialize EclipseData to DB column.", exc); - } - } - - @Override - public EclipseData convertToEntityAttribute(String raw) { - if (raw == null) - return null; - try { - return objectMapper.readValue(raw, EclipseData.class); - } catch (JsonProcessingException exc) { - throw new RuntimeException("Failed to parse EclipseData from DB column.", exc); - } - } - -} \ No newline at end of file diff --git a/server/src/main/java/org/eclipse/openvsx/entities/UserData.java b/server/src/main/java/org/eclipse/openvsx/entities/UserData.java index db2c9feb7..87d091560 100644 --- a/server/src/main/java/org/eclipse/openvsx/entities/UserData.java +++ b/server/src/main/java/org/eclipse/openvsx/entities/UserData.java @@ -52,9 +52,7 @@ public class UserData implements Serializable { @OneToMany(mappedBy = "user") List memberships; - @Column(length = 4096) - @Convert(converter = EclipseDataConverter.class) - EclipseData eclipseData; + String eclipsePersonId; @Column(length = 4096) @Convert(converter = AuthTokenConverter.class) @@ -150,12 +148,12 @@ public void setProviderUrl(String providerUrl) { this.providerUrl = providerUrl; } - public EclipseData getEclipseData() { - return eclipseData; + public String getEclipsePersonId() { + return eclipsePersonId; } - public void setEclipseData(EclipseData eclipseData) { - this.eclipseData = eclipseData; + public void setEclipsePersonId(String eclipsePersonId) { + this.eclipsePersonId = eclipsePersonId; } public AuthToken getGithubToken() { @@ -190,7 +188,7 @@ public boolean equals(Object o) { && Objects.equals(providerUrl, userData.providerUrl) && Objects.equals(tokens, userData.tokens) && Objects.equals(memberships, userData.memberships) - && Objects.equals(eclipseData, userData.eclipseData) + && Objects.equals(eclipsePersonId, userData.eclipsePersonId) && Objects.equals(githubToken, userData.githubToken) && Objects.equals(eclipseToken, userData.eclipseToken); } @@ -199,7 +197,7 @@ public boolean equals(Object o) { public int hashCode() { return Objects.hash( id, role, loginName, fullName, email, avatarUrl, provider, authId, providerUrl, tokens, memberships, - eclipseData, githubToken, eclipseToken + eclipsePersonId, githubToken, eclipseToken ); } } \ No newline at end of file diff --git a/server/src/main/java/org/eclipse/openvsx/security/OAuth2UserServices.java b/server/src/main/java/org/eclipse/openvsx/security/OAuth2UserServices.java index 66c86db04..b4c91bfe9 100644 --- a/server/src/main/java/org/eclipse/openvsx/security/OAuth2UserServices.java +++ b/server/src/main/java/org/eclipse/openvsx/security/OAuth2UserServices.java @@ -146,10 +146,8 @@ private IdPrincipal loadEclipseUser(OAuth2UserRequest userRequest) { + ") does not match your GitHub authentication (" + userData.getLoginName() + ").", ECLIPSE_MISMATCH_GITHUB_ID); + eclipse.updateUserData(userData, profile); - if (profile.publisherAgreements == null) { - eclipse.getPublisherAgreement(userData, accessToken); - } return principal; } catch (ErrorResultException exc) { throw new AuthenticationServiceException(exc.getMessage(), exc); diff --git a/server/src/main/resources/db/migration/V1_44__UserData_Add_EclipsePersonId.sql b/server/src/main/resources/db/migration/V1_44__UserData_Add_EclipsePersonId.sql new file mode 100644 index 000000000..e3fa017d9 --- /dev/null +++ b/server/src/main/resources/db/migration/V1_44__UserData_Add_EclipsePersonId.sql @@ -0,0 +1,3 @@ +ALTER TABLE user_data ADD COLUMN eclipse_person_id CHARACTER VARYING(255); +UPDATE user_data SET eclipse_person_id = eclipse_data::json->'personId'; +ALTER TABLE user_data DROP COLUMN eclipse_data; \ No newline at end of file diff --git a/server/src/test/java/org/eclipse/openvsx/eclipse/EclipseServiceTest.java b/server/src/test/java/org/eclipse/openvsx/eclipse/EclipseServiceTest.java index 96632836a..88157da3d 100644 --- a/server/src/test/java/org/eclipse/openvsx/eclipse/EclipseServiceTest.java +++ b/server/src/test/java/org/eclipse/openvsx/eclipse/EclipseServiceTest.java @@ -16,6 +16,7 @@ import java.io.IOException; import java.io.InputStreamReader; +import java.time.LocalDateTime; import java.util.Map; import io.micrometer.observation.ObservationRegistry; @@ -89,22 +90,6 @@ public void setup() { eclipse.eclipseApiUrl = "https://test.openvsx.eclipse.org/"; } - @Test - public void testGetPublicProfile() throws Exception { - var urlTemplate = "https://test.openvsx.eclipse.org/account/profile/{personId}"; - Mockito.when(restTemplate.exchange(eq(urlTemplate), eq(HttpMethod.GET), any(HttpEntity.class), eq(String.class), eq(Map.of("personId", "test")))) - .thenReturn(mockProfileResponse()); - - var profile = eclipse.getPublicProfile("test"); - - assertThat(profile).isNotNull(); - assertThat(profile.name).isEqualTo("test"); - assertThat(profile.githubHandle).isEqualTo("test"); - assertThat(profile.publisherAgreements).isNotNull(); - assertThat(profile.publisherAgreements.openVsx).isNotNull(); - assertThat(profile.publisherAgreements.openVsx.version).isEqualTo("1"); - } - @Test public void testGetUserProfile() throws Exception { Mockito.when(restTemplate.exchange(any(RequestEntity.class), eq(String.class))) @@ -123,9 +108,7 @@ public void testGetUserProfile() throws Exception { @Test public void testGetPublisherAgreement() throws Exception { var user = mockUser(); - var eclipseData = new EclipseData(); - user.setEclipseData(eclipseData); - eclipseData.personId = "test"; + user.setEclipsePersonId("test"); var urlTemplate = "https://test.openvsx.eclipse.org/openvsx/publisher_agreement/{personId}"; Mockito.when(restTemplate.exchange(eq(urlTemplate), eq(HttpMethod.GET), any(HttpEntity.class), eq(String.class), eq(Map.of("personId", "test")))) @@ -133,19 +116,16 @@ public void testGetPublisherAgreement() throws Exception { var agreement = eclipse.getPublisherAgreement(user); assertThat(agreement).isNotNull(); - assertThat(agreement.isActive).isTrue(); + assertThat(agreement.isActive).isEqualTo(true); assertThat(agreement.documentId).isEqualTo("abcd"); assertThat(agreement.version).isEqualTo("1"); - assertThat(agreement.timestamp).isNotNull(); - assertThat(agreement.timestamp.toString()).isEqualTo("2020-10-09T05:10:32"); + assertThat(agreement.timestamp).isEqualTo(LocalDateTime.of(2020, 10, 9, 5, 10, 32)); } @Test public void testGetPublisherAgreementNotFound() throws Exception { var user = mockUser(); - var eclipseData = new EclipseData(); - user.setEclipseData(eclipseData); - eclipseData.personId = "test"; + user.setEclipsePersonId("test"); var urlTemplate = "https://test.openvsx.eclipse.org/openvsx/publisher_agreement/{personId}"; Mockito.when(restTemplate.exchange(eq(urlTemplate), eq(HttpMethod.GET), any(HttpEntity.class), eq(String.class), eq(Map.of("personId", "test")))) @@ -172,17 +152,12 @@ public void testSignPublisherAgreement() throws Exception { Mockito.when(repositories.findAccessTokens(user)) .thenReturn(Streamable.empty()); - eclipse.signPublisherAgreement(user); - - assertThat(user.getEclipseData()).isNotNull(); - var ed = user.getEclipseData(); - assertThat(ed.personId).isEqualTo("test"); - assertThat(ed.publisherAgreement).isNotNull(); - assertThat(ed.publisherAgreement.isActive).isTrue(); - assertThat(ed.publisherAgreement.documentId).isEqualTo("abcd"); - assertThat(ed.publisherAgreement.version).isEqualTo("1"); - assertThat(ed.publisherAgreement.timestamp).isNotNull(); - assertThat(ed.publisherAgreement.timestamp.toString()).isEqualTo("2020-10-09T05:10:32"); + var agreement = eclipse.signPublisherAgreement(user); + assertThat(agreement).isNotNull(); + assertThat(agreement.isActive).isEqualTo(true); + assertThat(agreement.documentId).isEqualTo("abcd"); + assertThat(agreement.version).isEqualTo("1"); + assertThat(agreement.timestamp).isEqualTo(LocalDateTime.of(2020, 10, 9, 5, 10, 32)); } @Test @@ -208,9 +183,13 @@ public void testSignPublisherAgreementReactivateExtension() throws Exception { Mockito.when(repositories.findVersionsByAccessToken(accessToken, false)) .thenReturn(Streamable.of(extVersion)); - eclipse.signPublisherAgreement(user); + var agreement = eclipse.signPublisherAgreement(user); - assertThat(user.getEclipseData()).isNotNull(); + assertThat(agreement).isNotNull(); + assertThat(agreement.isActive).isEqualTo(true); + assertThat(agreement.documentId).isEqualTo("abcd"); + assertThat(agreement.version).isEqualTo("1"); + assertThat(agreement.timestamp).isEqualTo(LocalDateTime.of(2020, 10, 9, 5, 10, 32)); assertThat(extVersion.isActive()).isTrue(); assertThat(extension.isActive()).isTrue(); } @@ -230,27 +209,18 @@ public void testPublisherAgreementAlreadySigned() throws Exception { } @Test - public void testRevokePublisherAgreement() throws Exception { + public void testRevokePublisherAgreement() { var user = mockUser(); - var eclipseData = new EclipseData(); - user.setEclipseData(eclipseData); - eclipseData.personId = "test"; - eclipseData.publisherAgreement = new EclipseData.PublisherAgreement(); - eclipseData.publisherAgreement.isActive = true; + user.setEclipsePersonId("test"); eclipse.revokePublisherAgreement(user, null); - - assertThat(user.getEclipseData().publisherAgreement.isActive).isFalse(); } @Test - public void testRevokePublisherAgreementByAdmin() throws Exception { + public void testRevokePublisherAgreementByAdmin() { var user = mockUser(); - var eclipseData = new EclipseData(); - user.setEclipseData(eclipseData); - eclipseData.personId = "test"; - eclipseData.publisherAgreement = new EclipseData.PublisherAgreement(); - eclipseData.publisherAgreement.isActive = true; + user.setEclipsePersonId("test"); + var admin = new UserData(); admin.setLoginName("admin"); admin.setEclipseToken(new AuthToken()); @@ -259,8 +229,6 @@ public void testRevokePublisherAgreementByAdmin() throws Exception { .thenReturn(admin.getEclipseToken()); eclipse.revokePublisherAgreement(user, admin); - - assertThat(user.getEclipseData().publisherAgreement.isActive).isFalse(); } private UserData mockUser() { @@ -302,13 +270,12 @@ ObservationRegistry observationRegistry() { @Bean EclipseService eclipseService( TokenService tokens, - TransactionTemplate transactions, ExtensionService extensions, EntityManager entityManager, RestTemplate restTemplate, ObservationRegistry observations ) { - return new EclipseService(tokens, transactions, extensions, entityManager, restTemplate, observations); + return new EclipseService(tokens, extensions, entityManager, restTemplate, observations); } @Bean