Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
33 changes: 17 additions & 16 deletions server/src/main/java/org/eclipse/openvsx/ExtensionService.java
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,12 @@ private long getMaxContentSize() {
public ExtensionVersion mirrorVersion(
TempFile extensionFile,
String signatureName,
PersonalAccessToken token,
UserData user,
String binaryName,
String timestamp
) {
try (var processor = new ExtensionProcessor(extensionFile)) {
doPublish(processor, binaryName, token, TimeUtil.fromUTCString(timestamp), false);
doPublish(processor, binaryName, user, TimeUtil.fromUTCString(timestamp), false);
}
publishHandler.mirror(extensionFile, signatureName);
return extensionFile.getResource().getExtension();
Expand Down Expand Up @@ -128,28 +128,28 @@ public TempFile createExtensionFile(InputStream content) {
}
}

public ExtensionVersion publishVersion(InputStream inputStream, PersonalAccessToken token)
public ExtensionVersion publishVersion(InputStream inputStream, UserData user)
throws ErrorResultException {
try (
TempFile tempFile = createExtensionFile(inputStream);
ExtensionProcessor processor = new ExtensionProcessor(tempFile)
) {
return publishVersion(processor, token);
return publishVersion(processor, user);
} catch (IOException e) {
throw new ErrorResultException("Failed to read extension file", e);
}
}

public ExtensionVersion publishVersion(ExtensionProcessor processor, PersonalAccessToken token)
public ExtensionVersion publishVersion(ExtensionProcessor processor, UserData user)
throws ErrorResultException {
requireNonNull(processor);
requireNonNull(token);
requireNonNull(user);
var content = processor.getExtensionFile();
if (scanService.isEnabled()) {
return publishVersionWithScan(processor, token);
return publishVersionWithScan(processor, user);
} else {
try {
doPublish(processor, null, token, TimeUtil.getCurrentUTC(), true);
doPublish(processor, null, user, TimeUtil.getCurrentUTC(), true);
} catch (ErrorResultException exc) {
// In case publication fails early on we need to
// delete the temporary extension file, otherwise
Expand All @@ -164,7 +164,7 @@ public ExtensionVersion publishVersion(ExtensionProcessor processor, PersonalAcc
}
}

private ExtensionVersion publishVersionWithScan(ExtensionProcessor processor, PersonalAccessToken token)
private ExtensionVersion publishVersionWithScan(ExtensionProcessor processor, UserData user)
throws ErrorResultException {
var extensionFile = processor.getExtensionFile();
ExtensionScan scan = null;
Expand All @@ -173,13 +173,13 @@ private ExtensionVersion publishVersionWithScan(ExtensionProcessor processor, Pe
// Fail before any validation or scanning happens (and before a scan record is stored) if the
// extension version can not be published anyway, e.g. because the publisher lacks the access
// rights for the namespace or the version is published already.
publishHandler.checkPublishPreconditions(processor, token);
publishHandler.checkPublishPreconditions(processor, user);

scan = scanService.initializeScan(processor, token.getUser());
scan = scanService.initializeScan(processor, user);

scanService.runValidation(scan, extensionFile, token.getUser());
scanService.runValidation(scan, extensionFile, user);

doPublish(processor, null, token, TimeUtil.getCurrentUTC(), true);
doPublish(processor, null, user, TimeUtil.getCurrentUTC(), true);

// Publish async handles requesting the long-running scans
publishHandler.publishAsync(extensionFile, this, scan);
Expand Down Expand Up @@ -208,11 +208,12 @@ private ExtensionVersion publishVersionWithScan(ExtensionProcessor processor, Pe
private void doPublish(
ExtensionProcessor processor,
String binaryName,
PersonalAccessToken token,
UserData user,
LocalDateTime timestamp,
boolean checkDependencies
) {
var extVersion = publishHandler.createExtensionVersion(processor, token, timestamp, checkDependencies);
var extVersion = publishHandler
.createExtensionVersion(processor, user, timestamp, checkDependencies);
var download = processor.getBinary(extVersion, binaryName);
processor.getExtensionFile().setResource(download);
}
Expand Down Expand Up @@ -383,7 +384,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
46 changes: 27 additions & 19 deletions server/src/main/java/org/eclipse/openvsx/LocalRegistryService.java
Original file line number Diff line number Diff line change
Expand Up @@ -721,12 +721,12 @@ private Map<Long, List<NamespaceMembership>> getMemberships(Collection<Extension

@Transactional(rollbackOn = ErrorResultException.class)
public ResultJson createNamespace(NamespaceJson json, String tokenValue) {
var token = tokens.useAccessToken(tokenValue, new AccessTokenAction.CreateNamespace(json.getName()));
if (token == null) {
var user = tokens.useAccessToken(tokenValue, new AccessTokenAction.CreateNamespace(json.getName()));
if (user == null) {
throw new ErrorResultException(ACCESS_TOKEN_ERROR, HttpStatus.UNAUTHORIZED);
}

return createNamespace(json, token.getUser());
return createNamespace(json, user);
}

@Transactional(rollbackOn = ErrorResultException.class)
Expand Down Expand Up @@ -773,8 +773,8 @@ public ResultJson createNamespace(NamespaceJson json, UserData user) {
}

public ResultJson verifyToken(String namespaceName, String tokenValue) {
var token = tokens.useAccessToken(tokenValue, new AccessTokenAction.Verify());
if (token == null) {
var user = tokens.useAccessToken(tokenValue, new AccessTokenAction.Verify());
if (user == null) {
throw new ErrorResultException(ACCESS_TOKEN_ERROR, HttpStatus.UNAUTHORIZED);
}

Expand All @@ -783,7 +783,6 @@ public ResultJson verifyToken(String namespaceName, String tokenValue) {
throw new NotFoundException();
}

var user = token.getUser();
if (!users.hasPublishPermission(user, namespace)) {
throw new ErrorResultException(
"Insufficient access rights for namespace: " + namespace.getName(),
Expand All @@ -794,10 +793,15 @@ public ResultJson verifyToken(String namespaceName, String tokenValue) {
}

public ExtensionJson publish(InputStream content, UserData user) throws ErrorResultException {
return publish(content, tokens.createOneTimeAccessToken(user, "One time use publish token").getValue());
return publish(content, null, user);
}

public ExtensionJson publish(InputStream rawContent, String tokenValue) throws ErrorResultException {
return publish(rawContent, tokenValue, null);
}

private ExtensionJson publish(InputStream rawContent, String tokenValue, UserData user)
throws ErrorResultException {
// A rejection anywhere below - invalid/expired token, missing publisher agreement, or
// (pre-existing, inside extensions.publishVersion) an oversized package - can happen before
// the request body has been fully read. Wrapping it once here and relying on
Expand All @@ -812,17 +816,21 @@ public ExtensionJson publish(InputStream rawContent, String tokenValue) throws E
) {
ExtensionVersion extVersion;
try (var processor = new ExtensionProcessor(tempFile)) {
// now that we know the details, ensure token is still fine
var token = tokens.useAccessToken(
tokenValue,
new AccessTokenAction.PublishVersion(processor.getNamespace(), processor.getExtensionName()));
if (token == null || token.getUser() == null) {
throw new ErrorResultException(ACCESS_TOKEN_ERROR, HttpStatus.UNAUTHORIZED);
if (user == null) {
// now that we know the details, ensure token is still fine
user = tokens.useAccessToken(
tokenValue,
new AccessTokenAction.PublishVersion(
processor.getNamespace(),
processor.getExtensionName()));
if (user == null) {
throw new ErrorResultException(ACCESS_TOKEN_ERROR, HttpStatus.UNAUTHORIZED);
}
}
// Check whether the user has a valid publisher agreement
eclipse.checkPublisherAgreement(token.getUser());
eclipse.checkPublisherAgreement(user);

extVersion = extensions.publishVersion(processor, token);
extVersion = extensions.publishVersion(processor, user);
}

var json = toExtensionVersionJson(extVersion, null, true);
Expand Down Expand Up @@ -1302,11 +1310,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 @@ -1319,11 +1327,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 @@ -82,18 +82,6 @@ public AccessTokenJson createLongLivedAccessToken(UserData user, String descript
return createAccessToken(user, description, expiresTimestamp, null, null, null, PersonalAccessTokenType.LLT);
}

/**
* Creates a one-time usable token for user. Depending on configuration, the token expiration may be set as well.
*/
@Transactional
public AccessTokenJson createOneTimeAccessToken(UserData user, String description) {
requireNonNull(user);
final LocalDateTime expiresTimestamp = config.isOttTokenExpiryEnabled()
? TimeUtil.getCurrentUTC().plus(config.getOttExpiration())
: null;
return createAccessToken(user, description, expiresTimestamp, null, null, null, PersonalAccessTokenType.OTT);
}

/**
* Creates a trusted publishing token for a trusted publisher. The token is scoped to given trusted publisher
* associated extension only. Depending on configuration, the token expiration may be set as well.
Expand Down Expand Up @@ -193,7 +181,7 @@ public ResultJson deactivateAccessToken(UserData user, long id) {
// throws once this method returns null - silently discarding the fact that the token was touched
// or found expired.
@Transactional(TxType.REQUIRES_NEW)
public PersonalAccessToken useAccessToken(String tokenValue, AccessTokenAction accessTokenAction) {
public UserData useAccessToken(String tokenValue, AccessTokenAction accessTokenAction) {
var token = repositories.findPersonalAccessToken(hashTokenValue(tokenValue));
if (token == null) {
// assume DB contains token v0; fetch and upgrade if found active token
Expand Down Expand Up @@ -229,9 +217,10 @@ public PersonalAccessToken useAccessToken(String tokenValue, AccessTokenAction a
token.setAccessedTimestamp(now);
if (token.getType().isOneTime()) {
token.setActive(false);
entityManager.remove(token);
}
}
return token;
return token.getUser();
}

private AccessTokenScope getScope(PersonalAccessToken token) {
Expand Down
25 changes: 6 additions & 19 deletions server/src/main/java/org/eclipse/openvsx/admin/AdminService.java
Original file line number Diff line number Diff line change
Expand Up @@ -629,30 +629,18 @@ public ResultJson forgetUser(String provider, String username, UserData admin) {
removedCustomerMembershipCount++;
}

// Personal access tokens. Delete tokens that no retained extension version references;
// scrub and deactivate the rest so retained versions still resolve a publisher.
// Personal access tokens are no longer referenced by extension versions, so they can
// always be deleted outright.
var deletedTokenCount = 0;
var scrubbedTokenCount = 0;
for (var token : repositories.findPersonalAccessTokens(user)) {
if (repositories.countVersionsByAccessToken(token) == 0) {
entityManager.remove(token);
deletedTokenCount++;
} else {
token.setActive(false);
token.setDescription(null);
// The value is deliberately left in place: AccessTokenService.generateTokenValue()
// checks repositories.hasPersonalAccessToken(value) across all tokens, active or not, to
// avoid ever reissuing a value that was already handed out. Nulling it here would
// let that (astronomically unlikely) collision go undetected.
scrubbedTokenCount++;
}
entityManager.remove(token);
deletedTokenCount++;
}

// Namespace and customer memberships are already fully removed above. If nothing else in
// the database still refers to this user either, delete the row outright instead of
// anonymizing it.
var canDeleteUser = scrubbedTokenCount == 0
&& repositories.countReviews(user) == 0
var canDeleteUser = repositories.countReviews(user) == 0
&& repositories.countVersionsRemovedBy(user) == 0
&& repositories.countAdminScanDecisions(user) == 0
&& repositories.countFileDecisions(user) == 0
Expand Down Expand Up @@ -683,7 +671,7 @@ public ResultJson forgetUser(String provider, String username, UserData admin) {
+ removedExtensionCount + " extensions, removed "
+ removedMembershipCount + " namespace memberships, removed "
+ removedCustomerMembershipCount + " customer memberships, deleted "
+ deletedTokenCount + " tokens, scrubbed " + scrubbedTokenCount + " tokens.");
+ deletedTokenCount + " tokens.");
logs.logAction(admin, result);
return result;
}
Expand All @@ -695,7 +683,6 @@ public UserData checkAdminUser() {
public UserData checkAdminUser(String tokenValue) {
var user = Optional.of(tokenValue)
.map(tv -> tokens.useAccessToken(tv, new AccessTokenAction.Administration()))
.map(PersonalAccessToken::getUser)
.orElse(null);

return checkAdminUser(user);
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.findAllPersonalAccessTokens().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.findPublishersWithActiveVersions().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
Loading