Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ private List<ExtensionVersion> resolveVersions(
var versions = Arrays.stream(targetVersions)
.map(target -> {
var extVersion = restrictedToUser
? repositories.findVersionPublishedWithUser(
? repositories.findVersionPublishedByUser(
user,
target.version(),
target.targetPlatform(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1276,11 +1276,11 @@ private ExtensionReplacementJson toReplacementJson(
}

private boolean isVerified(ExtensionVersion extVersion) {
if (extVersion.getPublishedWith() == null) {
if (extVersion.getPublishedBy() == null) {
return false;
}

var user = extVersion.getPublishedWith().getUser();
var user = extVersion.getPublishedBy();
if (UserData.Role.PRIVILEGED.equals(user.getRole())) {
return true;
}
Expand All @@ -1293,11 +1293,11 @@ private boolean isVerified(
ExtensionVersion extVersion,
Map<Long, List<NamespaceMembership>> membershipsByNamespaceId
) {
if (extVersion.getPublishedWith() == null) {
if (extVersion.getPublishedBy() == null) {
return false;
}

var user = extVersion.getPublishedWith().getUser();
var user = extVersion.getPublishedBy();
if (UserData.Role.PRIVILEGED.equals(user.getRole())) {
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@
package org.eclipse.openvsx.eclipse;

import java.util.LinkedHashSet;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;

import jakarta.persistence.EntityManager;
import org.slf4j.Logger;
Expand All @@ -25,9 +23,7 @@

import org.eclipse.openvsx.ExtensionService;
import org.eclipse.openvsx.entities.Extension;
import org.eclipse.openvsx.entities.ExtensionVersionChange;
import org.eclipse.openvsx.entities.ExtensionVersionState;
import org.eclipse.openvsx.entities.PersonalAccessToken;
import org.eclipse.openvsx.entities.UserData;
import org.eclipse.openvsx.repositories.RepositoryService;
import org.eclipse.openvsx.util.NamingUtil;
Expand Down Expand Up @@ -67,14 +63,11 @@ public void checkPublishers(ApplicationStartedEvent event) {
return;
}

var publisherTokens = repositories.findAllAccessTokens().stream()
.collect(Collectors.groupingBy(PersonalAccessToken::getUser));
publisherTokens.keySet().forEach(user -> {
var accessTokens = publisherTokens.get(user);
if (!accessTokens.isEmpty() && !isCompliant(user)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: I think here we're now skipping an additional check that was made before where we would first check if the non compliant user had access tokens before performing the deactivation of its extensions.

Is that something we're doing on purpose?

repositories.findPublishers().forEach(user -> {
if (!isCompliant(user)) {
// Found a non-compliant publisher: deactivate all extension versions
transactions.<Void>execute(status -> {
deactivateExtensions(accessTokens);
deactivateExtensions(user);
return null;
});
}
Expand All @@ -100,25 +93,23 @@ private boolean isCompliant(UserData user) {
.isPresent();
}

private void deactivateExtensions(List<PersonalAccessToken> accessTokens) {
private void deactivateExtensions(UserData user) {
var affectedExtensions = new LinkedHashSet<Extension>();
var now = TimeUtil.getCurrentUTC();
for (var accessToken : accessTokens) {
var versions = repositories.findVersionsByAccessToken(accessToken, true);
for (var version : versions) {
version.setActive(false);
// the version stops being publicly visible here, which the changes feed reports at
// this instant rather than at the one it was published at
repositories.recordExtensionVersionChange(version, ExtensionVersionState.INACTIVE, now);
entityManager.merge(version);
var extension = version.getExtension();
affectedExtensions.add(extension);
logger.atInfo()
.setMessage("Deactivated: {} - {}")
.addArgument(() -> accessToken.getUser().getLoginName())
.addArgument(() -> NamingUtil.toLogFormat(version))
.log();
}
var versions = repositories.findVersionsByUser(user, true);
for (var version : versions) {
version.setActive(false);
// the version stops being publicly visible here, which the changes feed reports at
// this instant rather than at the one it was published at
repositories.recordExtensionVersionChange(version, ExtensionVersionState.INACTIVE, now);
entityManager.merge(version);
var extension = version.getExtension();
affectedExtensions.add(extension);
logger.atInfo()
.setMessage("Deactivated: {} - {}")
.addArgument(user::getLoginName)
.addArgument(() -> NamingUtil.toLogFormat(version))
.log();
}

// Update affected extensions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,13 @@ public enum Type {
@ManyToOne
private PersonalAccessToken publishedWith;

/**
* Who published this version. Recorded independently of {@code publishedWith} so that authorship
* survives the credential: a token can be revoked, expire or be deleted, and this stays.
*/
@ManyToOne
private UserData publishedBy;

private boolean active;

private boolean potentiallyMalicious;
Expand Down Expand Up @@ -197,8 +204,8 @@ public ExtensionJson toExtensionJson() {
json.setGalleryTheme(this.getGalleryTheme());
json.setLocalizedLanguages(this.getLocalizedLanguages());
json.setQna(this.getQna());
if (this.getPublishedWith() != null) {
json.setPublishedBy(this.getPublishedWith().getUser().toUserJson());
if (this.getPublishedBy() != null) {
json.setPublishedBy(this.getPublishedBy().toUserJson());
}
if (this.getDependencies() != null) {
json.setDependencies(toExtensionReferenceJson(this.getDependencies()));
Expand Down Expand Up @@ -336,6 +343,14 @@ public void setPublishedWith(PersonalAccessToken publishedWith) {
this.publishedWith = publishedWith;
}

public UserData getPublishedBy() {
return publishedBy;
}

public void setPublishedBy(UserData publishedBy) {
this.publishedBy = publishedBy;
}

public boolean isActive() {
return active;
}
Expand Down Expand Up @@ -569,7 +584,9 @@ public boolean equals(Object o) {
&& Objects.equals(version, that.version)
&& Objects.equals(targetPlatform, that.targetPlatform)
&& Objects.equals(timestamp, that.timestamp)
&& Objects.equals(getId(publishedWith), getId(that.publishedWith)) // use id to prevent infinite recursion && Objects.equals(displayName, that.displayName)
&& Objects.equals(getId(publishedWith), getId(that.publishedWith)) // use id to prevent infinite recursion
&& Objects.equals(getId(publishedBy), getId(that.publishedBy)) // use id to prevent infinite recursion
&& Objects.equals(displayName, that.displayName)
&& Objects.equals(description, that.description)
&& Objects.equals(engines, that.engines)
&& Objects.equals(categories, that.categories)
Expand Down Expand Up @@ -603,6 +620,7 @@ public int hashCode() {
preview,
timestamp,
getId(publishedWith),
getId(publishedBy),
active,
potentiallyMalicious,
removed,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ private LinkedHashSet<UserData> getExtensionPublishers(Streamable<Extension> ext
var publishers = new LinkedHashSet<UserData>();
for (var extension : extensions) {
for (var extVersion : repositories.findActiveVersions(extension)) {
if (extVersion.getPublishedWith() != null) {
publishers.add(extVersion.getPublishedWith().getUser());
if (extVersion.getPublishedBy() != null) {
publishers.add(extVersion.getPublishedBy());
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ private ExtensionVersion createExtensionVersion(

extVersion.setTimestamp(timestamp);
extVersion.setPublishedWith(token);
extVersion.setPublishedBy(user);
extVersion.setActive(false);

// Lock the extension row while adding a version so a concurrent delete-all serializes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,10 @@ public int countActiveExtensions() {
}

public int countActiveExtensionPublishers() {
var publishers = DSL.countDistinct(PERSONAL_ACCESS_TOKEN.USER_DATA);
var publishers = DSL.countDistinct(EXTENSION_VERSION.PUBLISHED_BY_ID);
return dsl.select(publishers)
.from(EXTENSION)
.join(EXTENSION_VERSION).on(EXTENSION_VERSION.EXTENSION_ID.eq(EXTENSION.ID))
.join(PERSONAL_ACCESS_TOKEN).on(PERSONAL_ACCESS_TOKEN.ID.eq(EXTENSION_VERSION.PUBLISHED_WITH_ID))
.where(EXTENSION.ACTIVE.eq(true))
.and(EXTENSION_VERSION.ACTIVE.eq(true))
.fetchOne(publishers);
Expand All @@ -62,14 +61,13 @@ public Map<Integer, Integer> countActiveExtensionPublishersGroupedByExtensionsPu
var aliasPublisher = "publisher";
var aliasExtensionCount = "extension_count";
var extensionCountsByPublisher = dsl.select(
PERSONAL_ACCESS_TOKEN.USER_DATA.as(aliasPublisher),
EXTENSION_VERSION.PUBLISHED_BY_ID.as(aliasPublisher),
DSL.countDistinct(EXTENSION.ID).as(aliasExtensionCount))
.from(EXTENSION)
.join(EXTENSION_VERSION).on(EXTENSION_VERSION.EXTENSION_ID.eq(EXTENSION.ID))
.join(PERSONAL_ACCESS_TOKEN).on(PERSONAL_ACCESS_TOKEN.ID.eq(EXTENSION_VERSION.PUBLISHED_WITH_ID))
.where(EXTENSION.ACTIVE.eq(true))
.and(EXTENSION_VERSION.ACTIVE.eq(true))
.groupBy(PERSONAL_ACCESS_TOKEN.USER_DATA)
.groupBy(EXTENSION_VERSION.PUBLISHED_BY_ID)
.asTable("aep");

return dsl.select(
Expand Down Expand Up @@ -130,10 +128,9 @@ public int countPublishersThatClaimedNamespaceOwnership() {
return dsl.select(count)
.from(EXTENSION)
.join(EXTENSION_VERSION).on(EXTENSION_VERSION.EXTENSION_ID.eq(EXTENSION.ID))
.join(PERSONAL_ACCESS_TOKEN).on(PERSONAL_ACCESS_TOKEN.ID.eq(EXTENSION_VERSION.PUBLISHED_WITH_ID))
.join(NAMESPACE_MEMBERSHIP)
.on(
NAMESPACE_MEMBERSHIP.USER_DATA.eq(PERSONAL_ACCESS_TOKEN.USER_DATA)
NAMESPACE_MEMBERSHIP.USER_DATA.eq(EXTENSION_VERSION.PUBLISHED_BY_ID)
.and(NAMESPACE_MEMBERSHIP.NAMESPACE.eq(EXTENSION.NAMESPACE_ID)))
.where(EXTENSION.ACTIVE.eq(true))
.and(EXTENSION_VERSION.ACTIVE.eq(true))
Expand All @@ -145,8 +142,7 @@ public Map<String, Integer> topMostActivePublishingUsers(int limit) {
var count = DSL.count(EXTENSION_VERSION.ID).as("extension_version_count");
return dsl.select(USER_DATA.ID, USER_DATA.LOGIN_NAME, count)
.from(EXTENSION_VERSION)
.join(PERSONAL_ACCESS_TOKEN).on(PERSONAL_ACCESS_TOKEN.ID.eq(EXTENSION_VERSION.PUBLISHED_WITH_ID))
.join(USER_DATA).on(USER_DATA.ID.eq(PERSONAL_ACCESS_TOKEN.USER_DATA))
.join(USER_DATA).on(USER_DATA.ID.eq(EXTENSION_VERSION.PUBLISHED_BY_ID))
.where(EXTENSION_VERSION.ACTIVE.eq(true))
.groupBy(USER_DATA.ID)
.orderBy(count.desc())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ Extension findByNameIgnoreCaseAndNamespaceNameIgnoreCaseForUpdateNoWait(

Streamable<Extension> findByIdIn(Collection<Long> extensionIds);

Streamable<Extension> findDistinctByVersionsPublishedWithUser(UserData user);
Streamable<Extension> findDistinctByVersionsPublishedBy(UserData user);

long count();

Expand Down
Loading