diff --git a/server/src/dev/resources/application.yml b/server/src/dev/resources/application.yml index 7993ed138..0ba9f1a11 100644 --- a/server/src/dev/resources/application.yml +++ b/server/src/dev/resources/application.yml @@ -150,7 +150,8 @@ ovsx: local: directory: /tmp/ovsx access-token: - prefix: dev_ovsxat_ # use a token prefix that clearly indicates that it's for development + prefix: dev_ovsx # use a token prefix that clearly indicates that it's for development; the kind of + # token (at_, tp_) is appended by the code, so no trailing separator here expiration: 0 # do not expire tokens in a dev environment notification: 0 mail: diff --git a/server/src/main/java/org/eclipse/openvsx/UserService.java b/server/src/main/java/org/eclipse/openvsx/UserService.java index 209451722..c2b526b42 100644 --- a/server/src/main/java/org/eclipse/openvsx/UserService.java +++ b/server/src/main/java/org/eclipse/openvsx/UserService.java @@ -161,7 +161,22 @@ public ResultJson removeNamespaceMember(Namespace namespace, UserData user) thro throw new ErrorResultException( "User " + user.getLoginName() + " is not a member of " + namespace.getName() + "."); } + return removeNamespaceMembership(membership); + } + + /** + * Removes a membership that the caller already holds, for callers walking a user's memberships rather + * than naming one. Nothing here can fail on a row that has since gone or been duplicated, which matters + * for {@code AdminService#revokePublisherContributions}: it revokes a whole publisher in one + * transaction, and one unremovable membership must not take the token and version deactivations with it. + */ + @Transactional(rollbackOn = ErrorResultException.class) + @CacheEvict(value = { CACHE_NAMESPACE_DETAILS_JSON }, key = "#membership.namespace.name") + public ResultJson removeNamespaceMembership(NamespaceMembership membership) { + var namespace = membership.getNamespace(); + var user = membership.getUser(); entityManager.remove(membership); + revokeTrustedPublishers(namespace, user); return ResultJson.success("Removed " + user.getLoginName() + " from namespace " + namespace.getName() + "."); } @@ -177,7 +192,11 @@ public ResultJson addNamespaceMember(Namespace namespace, UserData user, String if (role.equals(membership.getRole())) { throw new ErrorResultException("User " + user.getLoginName() + " already has the role " + role + "."); } + var wasOwner = NamespaceMembership.ROLE_OWNER.equals(membership.getRole()); membership.setRole(role); + if (wasOwner) { + revokeTrustedPublishers(namespace, user); + } return ResultJson.success( "Changed role of " + user.getLoginName() + " in " + namespace.getName() + " to " + role + "."); } @@ -189,6 +208,24 @@ public ResultJson addNamespaceMember(Namespace namespace, UserData user, String return ResultJson.success("Added " + user.getLoginName() + " as " + role + " of " + namespace.getName() + "."); } + /** + * Only a namespace owner may register a trusted publisher, so a registration must not outlive the ownership + * it was made under: whoever stops being an owner - by demotion or by leaving the namespace - loses the + * registrations they created there. Deleting them also invalidates the publishing tokens already issued + * under them, which a mere loss of publishing rights would not. + */ + private void revokeTrustedPublishers(Namespace namespace, UserData user) { + var trustedPublishers = repositories.findTrustedPublishersByNamespaceAndCreatedBy(namespace, user).toList(); + trustedPublishers.forEach(repositories::deleteTrustedPublisher); + if (!trustedPublishers.isEmpty()) { + logger.info( + "Deleted {} trusted publisher(s) of {} in namespace {}, who is no longer an owner of it", + trustedPublishers.size(), + user.getLoginName(), + namespace.getName()); + } + } + @Transactional(rollbackOn = { ErrorResultException.class, NotFoundException.class }) @CacheEvict(value = { CACHE_NAMESPACE_DETAILS_JSON }, key = "#details.name") public ResultJson updateNamespaceDetails(NamespaceDetailsJson details, UserData user) { diff --git a/server/src/main/java/org/eclipse/openvsx/accesstoken/AccessTokenConfig.java b/server/src/main/java/org/eclipse/openvsx/accesstoken/AccessTokenConfig.java index 1de3b3b8e..6a18940a1 100644 --- a/server/src/main/java/org/eclipse/openvsx/accesstoken/AccessTokenConfig.java +++ b/server/src/main/java/org/eclipse/openvsx/accesstoken/AccessTokenConfig.java @@ -34,30 +34,6 @@ public class AccessTokenConfig { @Value("#{'${ovsx.access-token.prefix:${ovsx.token-prefix:}}'}") private String prefix; - /** - * The expiration period for one time personal access tokens. The one time - * access token must be used in this period. - *

- * If {@code 0} is provided, the one time access tokens do not expire. - *

- * Property: {@code ovsx.access-token.ott-expiration} - * Default: {@code PT5M}, expires in 5 minutes - */ - @Value("${ovsx.access-token.ott-expiration:PT5M}") - private Duration ottExpiration; - - /** - * The expiration period for one time trusted publishing tokens. The one time - * access token must be used in this period. - *

- * If {@code 0} is provided, the one time trusted publishing tokens do not expire. - *

- * Property: {@code ovsx.access-token.tpt-expiration} - * Default: {@code PT5M}, expires in 5 minutes - */ - @Value("${ovsx.access-token.tpt-expiration:PT5M}") - private Duration tptExpiration; - /** * The expiration period for long-lived personal access tokens. *

@@ -155,22 +131,6 @@ public boolean isTokenExpiryEnabled() { return this.expiration; } - public boolean isOttTokenExpiryEnabled() { - return this.ottExpiration.isPositive(); - } - - public @NonNull Duration getOttExpiration() { - return ottExpiration; - } - - public boolean isTptTokenExpiryEnabled() { - return this.tptExpiration.isPositive(); - } - - public @NonNull Duration getTptExpiration() { - return tptExpiration; - } - public boolean isTokenExpiryNotificationEnabled() { return this.notification.isPositive(); } @@ -222,14 +182,6 @@ public void validate() { throw new IllegalArgumentException( "ovsx.access-token.expiration must be a non-negative duration, got: " + expiration); } - if (ottExpiration.isNegative()) { - throw new IllegalArgumentException( - "ovsx.access-token.ott-expiration must be a non-negative duration, got: " + ottExpiration); - } - if (tptExpiration.isNegative()) { - throw new IllegalArgumentException( - "ovsx.access-token.tpt-expiration must be a non-negative duration, got: " + tptExpiration); - } if (notification.isNegative()) { throw new IllegalArgumentException( diff --git a/server/src/main/java/org/eclipse/openvsx/accesstoken/AccessTokenService.java b/server/src/main/java/org/eclipse/openvsx/accesstoken/AccessTokenService.java index ec2257ef6..f2f9421c3 100644 --- a/server/src/main/java/org/eclipse/openvsx/accesstoken/AccessTokenService.java +++ b/server/src/main/java/org/eclipse/openvsx/accesstoken/AccessTokenService.java @@ -15,13 +15,23 @@ import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; +import java.security.SecureRandom; +import java.time.Duration; import java.time.LocalDateTime; +import java.util.Arrays; +import java.util.Base64; +import java.util.List; +import java.util.Map; import jakarta.persistence.EntityManager; import jakarta.transaction.Transactional; import org.apache.commons.codec.binary.Hex; import org.apache.commons.codec.digest.DigestUtils; +import org.jooq.DSLContext; +import org.jooq.impl.DSL; import org.jspecify.annotations.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.stereotype.Service; import org.eclipse.openvsx.entities.Extension; @@ -36,7 +46,6 @@ import org.eclipse.openvsx.repositories.RepositoryService; import org.eclipse.openvsx.util.NotFoundException; import org.eclipse.openvsx.util.TimeUtil; -import org.eclipse.openvsx.util.UUIDService; import org.eclipse.openvsx.util.UrlUtil; import org.eclipse.openvsx.util.auth.AccessTokenAuthentication; @@ -46,29 +55,50 @@ @Service public class AccessTokenService { + /** + * Arbitrary, fixed key for the Postgres advisory lock guarding the token upgrade below. Must stay + * distinct from every other advisory lock key this application uses; see + * {@code ExtensionScanJobRecoveryService.RECOVERY_LOCK_KEY} for the other one. + */ + private static final long UPGRADE_LOCK_KEY = 891_234_567_890_124L; + + private static final Logger logger = LoggerFactory.getLogger(AccessTokenService.class); + + /** 256 bits; far beyond guessing, and the encoded form is still shorter than a UUID. */ + private static final int TOKEN_BYTES = 32; + + private static final Base64.Encoder TOKEN_ENCODER = Base64.getUrlEncoder().withoutPadding(); + + /** The token types that are retired by deleting the row rather than deactivating it. */ + private static final List ONE_TIME_TOKEN_TYPES = Arrays + .stream(PersonalAccessTokenType.values()) + .filter(PersonalAccessTokenType::isOneTime) + .toList(); + private static final int TOKEN_VERSION_0 = 0; private static final int TOKEN_VERSION_1 = 1; private static final int[] ALL_TOKEN_VERSIONS = { TOKEN_VERSION_0, TOKEN_VERSION_1 }; private static final int TOKEN_CURRENT_VERSION = TOKEN_VERSION_1; private final AccessTokenConfig config; - private final UUIDService uuidService; private final EntityManager entityManager; private final RepositoryService repositories; private final MailService mail; + private final DSLContext dsl; + private final SecureRandom random = new SecureRandom(); public AccessTokenService( AccessTokenConfig config, - UUIDService uuidService, EntityManager entityManager, RepositoryService repositories, - MailService mail + MailService mail, + DSLContext dsl ) { this.config = config; - this.uuidService = uuidService; this.entityManager = entityManager; this.repositories = repositories; this.mail = mail; + this.dsl = dsl; } /** @@ -80,24 +110,39 @@ public AccessTokenJson createLongLivedAccessToken(UserData user, String descript final LocalDateTime expiresTimestamp = config.isTokenExpiryEnabled() ? TimeUtil.getCurrentUTC().plus(config.getExpiration()) : null; - return createAccessToken(user, description, expiresTimestamp, null, null, null, PersonalAccessTokenType.LLT); + return createAccessToken( + user, + description, + expiresTimestamp, + null, + null, + null, + null, + PersonalAccessTokenType.LLT); } /** * 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. + * associated extension only. How long it lives is the trusted publishing configuration's to decide, so the + * caller passes it in rather than this service reading it. */ @Transactional - public AccessTokenJson createTrustedPublishingAccessToken(TrustedPublisher trustedPublisher, String description) { + public AccessTokenJson createTrustedPublishingAccessToken( + TrustedPublisher trustedPublisher, + String description, + Duration expiration, + Map claims + ) { requireNonNull(trustedPublisher); - final LocalDateTime expiresTimestamp = config.isTptTokenExpiryEnabled() - ? TimeUtil.getCurrentUTC().plus(config.getTptExpiration()) - : null; + requireNonNull(expiration); + requireNonNull(claims); + final LocalDateTime expiresTimestamp = TimeUtil.getCurrentUTC().plus(expiration); return createAccessToken( trustedPublisher.getCreatedBy(), description, expiresTimestamp, trustedPublisher, + claims, null, null, PersonalAccessTokenType.TPT); @@ -108,11 +153,12 @@ private AccessTokenJson createAccessToken( String description, @Nullable LocalDateTime expiresTimestamp, @Nullable TrustedPublisher trustedPublisher, + @Nullable Map claims, @Nullable Extension scopeExtension, @Nullable Namespace scopeNamespace, PersonalAccessTokenType type ) { - var rawValue = generateTokenValue(); + var rawValue = generateTokenValue(type); var token = new PersonalAccessToken(); token.setUser(user); token.setValue(hashTokenValue(rawValue)); @@ -127,8 +173,9 @@ private AccessTokenJson createAccessToken( if (type != PersonalAccessTokenType.TPT) { throw new IllegalArgumentException("Only TPT token may be created with TP"); } - // link TP and scope to TP.ext + // link TP and scope to TP.ext, and carry the exchange's claims to the publish that uses this token.setTrustedPublisher(trustedPublisher); + token.setClaims(claims); token.setScopeExtension(trustedPublisher.getExtension()); } else if (scopeExtension != null) { // scope to ext @@ -150,13 +197,21 @@ private AccessTokenJson createAccessToken( return json; } + /** + * Generates a token value: the deployment's prefix, the marker saying which kind of token this is, and + * 32 bytes from a CSPRNG. A UUID would be the wrong shape here - it is an identifier type, and the + * time-ordered v7 this application generates elsewhere spends 48 of its bits on a readable timestamp, + * leaving 74 random ones where raw bytes give 256. + *

+ * Uniqueness is the {@code UNIQUE (value)} constraint's to enforce, not this method's. It is the only + * check that can work: it applies to the hash that actually gets stored, and it holds across every pod + * writing to the database, which a check-then-insert here could not. + */ // public to be accessible from tests - public String generateTokenValue() { - String value; - do { - value = config.getPrefix() + uuidService.generateRandom(); - } while (repositories.hasPersonalAccessToken(value)); - return value; + public String generateTokenValue(PersonalAccessTokenType type) { + var bytes = new byte[TOKEN_BYTES]; + random.nextBytes(bytes); + return config.getPrefix() + type.getTokenMarker() + TOKEN_ENCODER.encodeToString(bytes); } @Transactional @@ -208,12 +263,19 @@ public AccessTokenAuthentication useAccessToken(String tokenValue, AccessTokenAc // findByExpiresTimestampLessThanEqual...: a token expiring at exactly `now` is expired. LocalDateTime now = TimeUtil.getCurrentUTC(); if (token.getExpiresTimestamp() != null && !token.getExpiresTimestamp().isAfter(now)) { - token.setActive(false); + if (token.getType().isOneTime()) { + entityManager.remove(token); + } else { + token.setActive(false); + } return null; } - // TPT without TP => registration was deleted + // Deleting a registration takes its tokens with it, so this should not be reachable; kept as a + // guard, because a TPT that lost its registration may only ever publish an extension it can no + // longer be checked against. Removed rather than deactivated: it can never become valid again, + // and nothing reads the row afterwards - the same reasoning as for a one-time token below. if (token.getType() == PersonalAccessTokenType.TPT && token.getTrustedPublisher() == null) { - token.setActive(false); + entityManager.remove(token); return null; } // scope @@ -229,7 +291,7 @@ public AccessTokenAuthentication useAccessToken(String tokenValue, AccessTokenAc entityManager.remove(token); } } - return new AccessTokenAuthentication(token.getUser(), token.getType()); + return new AccessTokenAuthentication(token.getUser(), token.getType(), token.getId(), token.getClaims()); } private AccessTokenScope getScope(PersonalAccessToken token) { @@ -242,9 +304,22 @@ private AccessTokenScope getScope(PersonalAccessToken token) { } } + /** + * Retires every token that has expired. + *

+ * A one-time token is deleted rather than deactivated, the same as when one is used, when its trusted + * publisher registration goes, or when its extension is purged: it can never be used again, nothing + * reads the row afterwards, and one is minted per exchange. Keeping the unused ones while deleting the + * used ones would retain exactly the rows nobody has a question about. + *

+ * Long-lived tokens stay as deactivated rows: a user is shown their own expired tokens, and the + * expiry notification mails read them. + */ @Transactional public int expireAccessTokens() { - var expiredAccessTokens = repositories.expirePersonalAccessTokens(TimeUtil.getCurrentUTC()); + var now = TimeUtil.getCurrentUTC(); + var deletedAccessTokens = repositories.deleteExpiredPersonalAccessTokens(now, ONE_TIME_TOKEN_TYPES); + var expiredAccessTokens = repositories.expirePersonalAccessTokens(now); if (config.isSendExpiredMailEnabled()) { for (var token : expiredAccessTokens) { if (token.getType().isNotify()) { @@ -252,7 +327,7 @@ public int expireAccessTokens() { } } } - return expiredAccessTokens.size(); + return deletedAccessTokens.size() + expiredAccessTokens.size(); } @Transactional @@ -272,8 +347,24 @@ public int setExpirationTimeForLegacyAccessTokens(LocalDateTime expirationTime) return repositories.updateExpiresTimeForLegacyPersonalAccessTokens(expirationTime, PersonalAccessTokenType.LLT); } + /** + * Upgrades every token still stored in a legacy format. + *

+ * The job that calls this is enqueued from {@code ApplicationStartedEvent}, which fires in every + * instance's own JVM: during a rolling update several pods run this against the same rows. The work + * itself is idempotent - each row is hashed from the raw value it still holds, and the row stops + * matching once upgraded - but there is no point in every pod scanning and rewriting the whole set, + * so a transaction-scoped advisory lock lets one of them do it. {@code pg_try_advisory_xact_lock} + * returns immediately rather than blocking, and Postgres drops the lock when this method's + * transaction ends, so there is no unlock to forget and none can leak onto a pooled connection. + */ @Transactional public int upgradeTokens() { + if (!tryAcquireUpgradeLock()) { + logger.debug("Another instance already holds the token upgrade lock, skipping"); + return 0; + } + int upgradedCount = 0; for (int version : ALL_TOKEN_VERSIONS) { if (version == TOKEN_CURRENT_VERSION) { @@ -289,6 +380,14 @@ public int upgradeTokens() { return upgradedCount; } + // Package-private so a test can stub it via a spy without needing two real database connections. + boolean tryAcquireUpgradeLock() { + return Boolean.TRUE.equals( + dsl.fetchValue( + DSL.select( + DSL.function("pg_try_advisory_xact_lock", Boolean.class, DSL.val(UPGRADE_LOCK_KEY))))); + } + private boolean upgradeToken(PersonalAccessToken token) { int version = token.getVersion(); if (version == TOKEN_VERSION_0) { 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 020aed491..8e1b95bd3 100644 --- a/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java +++ b/server/src/main/java/org/eclipse/openvsx/admin/AdminService.java @@ -536,13 +536,19 @@ public ResultJson revokePublisherContributions(String provider, String loginName extensions.updateExtension(extension); } - // revoke namespace memberships + // Revoke namespace memberships one by one through UserService rather than deleting them in bulk: + // that is what deletes the trusted publishers registered under an ownership being revoked here, and + // what evicts the namespace details cache - same as when an owner removes a member themselves. Each + // membership is handed over as the row we already hold, so no second lookup can fail and abort the + // whole revoke. var namespaceMemberships = repositories.findMemberships(user); var numberOfNamespaceMemberships = 0L; // add a null check due to tests using mocks which return null if (namespaceMemberships != null) { - numberOfNamespaceMemberships = namespaceMemberships.stream().count(); - repositories.deleteMemberships(user); + for (var membership : namespaceMemberships.toList()) { + users.removeNamespaceMembership(membership); + numberOfNamespaceMemberships++; + } } var message = "Deactivated " + deactivatedTokenCount + " tokens, " @@ -606,7 +612,7 @@ public ResultJson forgetUser(String provider, String username, UserData admin) { var removedMembershipCount = 0; for (var membership : repositories.findMemberships(user)) { var namespace = membership.getNamespace(); - users.removeNamespaceMember(namespace, user); + users.removeNamespaceMembership(membership); removedMembershipCount++; search.updateSearchEntries(repositories.findActiveExtensions(namespace).toList()); } diff --git a/server/src/main/java/org/eclipse/openvsx/entities/ExtensionVersion.java b/server/src/main/java/org/eclipse/openvsx/entities/ExtensionVersion.java index 09a467def..c8bfdf432 100644 --- a/server/src/main/java/org/eclipse/openvsx/entities/ExtensionVersion.java +++ b/server/src/main/java/org/eclipse/openvsx/entities/ExtensionVersion.java @@ -29,6 +29,9 @@ import jakarta.persistence.Table; import jakarta.persistence.Transient; import org.apache.commons.lang3.StringUtils; +import org.hibernate.annotations.JdbcTypeCode; +import org.hibernate.type.SqlTypes; +import org.jspecify.annotations.Nullable; import org.eclipse.openvsx.json.ExtensionJson; import org.eclipse.openvsx.json.ExtensionReferenceJson; @@ -92,6 +95,28 @@ public enum Type { @Enumerated(EnumType.STRING) private PersonalAccessTokenType publishedWithTt; + /** + * The token this version was published with, as best-effort provenance: it answers "what did this + * credential publish" after a leak. Nullable and allowed to decay - the token row is deleted when a + * one-time token is used or a forgotten user's tokens go - which is why authorship is recorded + * separately in {@link #publishedBy} and {@link #publishedWithTt} instead of being read through it. + */ + @Column(name = "published_with_id") + @Nullable + private Long publishedWithId; + + /** + * The OIDC identity that produced this version, for versions published through trusted publishing: the + * immutable repository and owner ids and the workflow reference including the ref it ran on, as the + * provider asserted them at the exchange. Null for everything published with an ordinary token. + *

+ * Copied at publish time rather than reached through the token, which is deleted as it is used. + */ + @JdbcTypeCode(SqlTypes.JSON) + @Column(name = "published_provenance", columnDefinition = "jsonb") + @Nullable + private Map publishedProvenance; + private boolean active; private boolean potentiallyMalicious; @@ -209,7 +234,8 @@ public ExtensionJson toExtensionJson() { if (this.getPublishedBy() != null) { json.setPublishedBy(this.getPublishedBy().toUserJson()); } - json.setTrustedPublisher(getPublishedWithTt() != null && getPublishedWithTt() == PersonalAccessTokenType.TPT); + json.setPublishedWithTrustedPublishing( + getPublishedWithTt() != null && getPublishedWithTt() == PersonalAccessTokenType.TPT); if (this.getDependencies() != null) { json.setDependencies(toExtensionReferenceJson(this.getDependencies())); } @@ -350,6 +376,24 @@ public PersonalAccessTokenType getPublishedWithTt() { return publishedWithTt; } + @Nullable + public Map getPublishedProvenance() { + return publishedProvenance; + } + + public void setPublishedProvenance(@Nullable Map publishedProvenance) { + this.publishedProvenance = publishedProvenance; + } + + @Nullable + public Long getPublishedWithId() { + return publishedWithId; + } + + public void setPublishedWithId(@Nullable Long publishedWithId) { + this.publishedWithId = publishedWithId; + } + public void setPublishedWithTt(PersonalAccessTokenType publishedWithTt) { this.publishedWithTt = publishedWithTt; } diff --git a/server/src/main/java/org/eclipse/openvsx/entities/PersonalAccessToken.java b/server/src/main/java/org/eclipse/openvsx/entities/PersonalAccessToken.java index 0333c4f6e..794e8a4c5 100644 --- a/server/src/main/java/org/eclipse/openvsx/entities/PersonalAccessToken.java +++ b/server/src/main/java/org/eclipse/openvsx/entities/PersonalAccessToken.java @@ -12,16 +12,27 @@ import java.io.Serial; import java.io.Serializable; import java.time.LocalDateTime; +import java.util.Map; import java.util.Objects; import jakarta.persistence.*; +import org.hibernate.annotations.DynamicUpdate; +import org.hibernate.annotations.JdbcTypeCode; +import org.hibernate.type.SqlTypes; +import org.jspecify.annotations.Nullable; import org.eclipse.openvsx.json.AccessTokenJson; import org.eclipse.openvsx.util.TimeUtil; import static java.util.Objects.requireNonNull; +// Same reasoning as on Extension: the paths that write a token row each touch a different column - +// the upgrade job rewrites `value` and `version`, using a token writes `accessed_timestamp`, revoking +// one writes `active`. Without @DynamicUpdate, Hibernate's full-row UPDATE would rewrite all of them +// from whatever the writing transaction happened to load, so an upgrade or a token use running +// alongside a revoke could silently put `active` back to true. @Entity +@DynamicUpdate @Table(name = "personal_access_token") public class PersonalAccessToken implements Serializable { @@ -71,6 +82,15 @@ public class PersonalAccessToken implements Serializable { @JoinColumn(name = "trusted_publisher_id") private TrustedPublisher trustedPublisher; + /** + * The OIDC claims this token was exchanged for, carried from the exchange to the publish that uses it + * and copied onto the version there. Null for every token that is not a trusted publishing one. + */ + @JdbcTypeCode(SqlTypes.JSON) + @Column(columnDefinition = "jsonb") + @Nullable + private Map claims; + /** * Convert to a JSON object. */ @@ -196,6 +216,15 @@ public void setScopeNamespace(Namespace scopeNamespace) { this.scopeNamespace = scopeNamespace; } + @Nullable + public Map getClaims() { + return claims; + } + + public void setClaims(@Nullable Map claims) { + this.claims = claims; + } + public TrustedPublisher getTrustedPublisher() { return trustedPublisher; } diff --git a/server/src/main/java/org/eclipse/openvsx/entities/PersonalAccessTokenType.java b/server/src/main/java/org/eclipse/openvsx/entities/PersonalAccessTokenType.java index 5d0422912..256fe8a68 100644 --- a/server/src/main/java/org/eclipse/openvsx/entities/PersonalAccessTokenType.java +++ b/server/src/main/java/org/eclipse/openvsx/entities/PersonalAccessTokenType.java @@ -19,26 +19,37 @@ public enum PersonalAccessTokenType { /** * Long-lived personal access token (classic). */ - LLT(false, true), + LLT("at_", false, true), /** * One time usable general personal access token. * Legacy: was used until 1.2.0; but is not anymore. */ @Deprecated - OTT(true, false), + OTT("ot_", true, false), /** * One time usable, trusted publishing issued access token. */ - TPT(true, false); + TPT("tp_", true, false); + private final String tokenMarker; private final boolean oneTime; private final boolean notify; - PersonalAccessTokenType(boolean oneTime, boolean notify) { + PersonalAccessTokenType(String tokenMarker, boolean oneTime, boolean notify) { + this.tokenMarker = tokenMarker; this.oneTime = oneTime; this.notify = notify; } + /** + * Marks which kind of token a value is, so that a leaked one says what it can do at a glance and + * secret scanning can tell them apart. It follows the deployment's own token prefix, which is where + * the registry names itself. + */ + public String getTokenMarker() { + return tokenMarker; + } + public boolean isOneTime() { return oneTime; } diff --git a/server/src/main/java/org/eclipse/openvsx/json/ExtensionJson.java b/server/src/main/java/org/eclipse/openvsx/json/ExtensionJson.java index 3bb3ae4df..0c7638bef 100644 --- a/server/src/main/java/org/eclipse/openvsx/json/ExtensionJson.java +++ b/server/src/main/java/org/eclipse/openvsx/json/ExtensionJson.java @@ -86,9 +86,9 @@ public static ExtensionJson error(String message) { @NotNull private UserJson publishedBy; - @Schema(description = "Indicates whether this version was published using trusted publisher") + @Schema(description = "Indicates whether this version was published using trusted publishing") @NotNull - private Boolean trustedPublisher; + private Boolean publishedWithTrustedPublishing; @Schema(hidden = true) private Boolean active; @@ -307,12 +307,12 @@ public void setPublishedBy(UserJson publishedBy) { this.publishedBy = publishedBy; } - public Boolean getTrustedPublisher() { - return trustedPublisher; + public Boolean getPublishedWithTrustedPublishing() { + return publishedWithTrustedPublishing; } - public void setTrustedPublisher(Boolean trustedPublisher) { - this.trustedPublisher = trustedPublisher; + public void setPublishedWithTrustedPublishing(Boolean publishedWithTrustedPublishing) { + this.publishedWithTrustedPublishing = publishedWithTrustedPublishing; } public Boolean getActive() { diff --git a/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandler.java b/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandler.java index af47545e8..62d9e6cc7 100644 --- a/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandler.java +++ b/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandler.java @@ -250,6 +250,8 @@ private ExtensionVersion createExtensionVersion( extVersion.setPublishedBy(au.userData()); if (au instanceof AccessTokenAuthentication ata) { extVersion.setPublishedWithTt(ata.type()); + extVersion.setPublishedWithId(ata.tokenId()); + extVersion.setPublishedProvenance(ata.claims()); } extVersion.setActive(false); diff --git a/server/src/main/java/org/eclipse/openvsx/repositories/PersonalAccessTokenRepository.java b/server/src/main/java/org/eclipse/openvsx/repositories/PersonalAccessTokenRepository.java index 46fc0f9e5..83a2c873d 100644 --- a/server/src/main/java/org/eclipse/openvsx/repositories/PersonalAccessTokenRepository.java +++ b/server/src/main/java/org/eclipse/openvsx/repositories/PersonalAccessTokenRepository.java @@ -10,6 +10,7 @@ package org.eclipse.openvsx.repositories; import java.time.LocalDateTime; +import java.util.Collection; import java.util.List; import org.springframework.data.domain.Pageable; @@ -61,4 +62,15 @@ List findByExpiresTimestampLessThanEqualAndActiveTrueAndNot nativeQuery = true ) List expireAccessTokens(LocalDateTime timestamp); + + /** + * Deletes the expired tokens of the given types outright. A one-time token that expired unused can + * never be used, and nothing reads the row afterwards - the same reason using one deletes it. + */ + @Modifying + @Query( + value = "delete from personal_access_token where expires_timestamp <= ?1 and type in ?2 returning *", + nativeQuery = true + ) + List deleteExpiredAccessTokens(LocalDateTime timestamp, Collection types); } diff --git a/server/src/main/java/org/eclipse/openvsx/repositories/RepositoryService.java b/server/src/main/java/org/eclipse/openvsx/repositories/RepositoryService.java index 4f8cc866f..4449338db 100644 --- a/server/src/main/java/org/eclipse/openvsx/repositories/RepositoryService.java +++ b/server/src/main/java/org/eclipse/openvsx/repositories/RepositoryService.java @@ -195,6 +195,13 @@ public Streamable findTrustedPublishersByExtension(Extension e return trustedPublisherRepo.findTrustedPublishersByExtension(extension); } + public Streamable findTrustedPublishersByNamespaceAndCreatedBy( + Namespace namespace, + UserData createdBy + ) { + return trustedPublisherRepo.findByExtension_NamespaceAndCreatedBy(namespace, createdBy); + } + public TrustedPublisher findTrustedPublisher(long id) { return trustedPublisherRepo.findById(id); } @@ -479,10 +486,6 @@ public NamespaceMembership findMembership(UserData user, Namespace namespace) { return membershipRepo.findByUserAndNamespace(user, namespace); } - public void deleteMemberships(UserData user) { - membershipRepo.deleteByUser(user); - } - public boolean hasMembership(UserData user, Namespace namespace) { return membershipJooqRepo.hasMembership(user, namespace); } @@ -587,12 +590,16 @@ public List expirePersonalAccessTokens(LocalDateTime timest return personalAccessTokenRepo.expireAccessTokens(timestamp); } - public int updateExpiresTimeForLegacyPersonalAccessTokens(LocalDateTime timestamp, PersonalAccessTokenType type) { - return personalAccessTokenRepo.updateExpiresTimeForLegacyAccessTokens(timestamp, type); + public List deleteExpiredPersonalAccessTokens( + LocalDateTime timestamp, + Collection types + ) { + return personalAccessTokenRepo + .deleteExpiredAccessTokens(timestamp, types.stream().map(Enum::name).toList()); } - public boolean hasPersonalAccessToken(String value) { - return personalAccessTokenRepo.findByValue(value) != null; + public int updateExpiresTimeForLegacyPersonalAccessTokens(LocalDateTime timestamp, PersonalAccessTokenType type) { + return personalAccessTokenRepo.updateExpiresTimeForLegacyAccessTokens(timestamp, type); } public PersonalAccessToken findPersonalAccessToken(UserData user, String description) { diff --git a/server/src/main/java/org/eclipse/openvsx/repositories/TrustedPublisherRepository.java b/server/src/main/java/org/eclipse/openvsx/repositories/TrustedPublisherRepository.java index 22e9882e4..e51f6fd67 100644 --- a/server/src/main/java/org/eclipse/openvsx/repositories/TrustedPublisherRepository.java +++ b/server/src/main/java/org/eclipse/openvsx/repositories/TrustedPublisherRepository.java @@ -16,11 +16,15 @@ import org.springframework.data.util.Streamable; import org.eclipse.openvsx.entities.Extension; +import org.eclipse.openvsx.entities.Namespace; import org.eclipse.openvsx.entities.TrustedPublisher; +import org.eclipse.openvsx.entities.UserData; public interface TrustedPublisherRepository extends Repository { Streamable findTrustedPublishersByExtension(Extension extension); + Streamable findByExtension_NamespaceAndCreatedBy(Namespace namespace, UserData createdBy); + TrustedPublisher findById(long id); void delete(TrustedPublisher trustedPublisher); diff --git a/server/src/main/java/org/eclipse/openvsx/TrustedPublishingAPI.java b/server/src/main/java/org/eclipse/openvsx/trustedpublishing/TrustedPublishingAPI.java similarity index 89% rename from server/src/main/java/org/eclipse/openvsx/TrustedPublishingAPI.java rename to server/src/main/java/org/eclipse/openvsx/trustedpublishing/TrustedPublishingAPI.java index 22ac3406d..af4ccf731 100644 --- a/server/src/main/java/org/eclipse/openvsx/TrustedPublishingAPI.java +++ b/server/src/main/java/org/eclipse/openvsx/trustedpublishing/TrustedPublishingAPI.java @@ -10,7 +10,7 @@ * * SPDX-License-Identifier: EPL-2.0 *****************************************************************************/ -package org.eclipse.openvsx; +package org.eclipse.openvsx.trustedpublishing; import java.util.Objects; @@ -25,6 +25,7 @@ import org.springframework.web.bind.annotation.RestController; import org.springframework.web.server.ResponseStatusException; +import org.eclipse.openvsx.UserService; import org.eclipse.openvsx.eclipse.EclipseService; import org.eclipse.openvsx.entities.TrustedPublisher; import org.eclipse.openvsx.json.AccessTokenJson; @@ -35,7 +36,6 @@ import org.eclipse.openvsx.json.TrustedPublisherStatusJson; import org.eclipse.openvsx.json.TrustedPublisherTokenRequestJson; import org.eclipse.openvsx.settings.MutatingOperation; -import org.eclipse.openvsx.trustedpublishing.TrustedPublishingService; import org.eclipse.openvsx.util.ErrorResultException; import org.eclipse.openvsx.util.NotFoundException; @@ -69,20 +69,21 @@ public ResponseEntity createTrustedPublisher( if (user == null) { throw new ResponseStatusException(HttpStatus.FORBIDDEN); } - eclipseService.checkPublisherAgreement(user); - if (!StringUtils.hasText(request.getProvider()) - || !StringUtils.hasText(request.getNamespace()) || !StringUtils.hasText(request.getExtension()) - || request.getRegistration() == null || request.getRegistration().isEmpty()) { - var json = TrustedPublisherJson - .error("The fields provider, namespace, extension and registration are mandatory."); - return new ResponseEntity<>(json, HttpStatus.BAD_REQUEST); - } - if (!Objects.equals(namespace, request.getNamespace())) { - var json = TrustedPublisherJson.error("The namespace in the path and in the request body must match."); - return new ResponseEntity<>(json, HttpStatus.BAD_REQUEST); - } - try { + // throws when the user has no signed publisher agreement, and so has to be caught below + eclipseService.checkPublisherAgreement(user); + if (!StringUtils.hasText(request.getProvider()) + || !StringUtils.hasText(request.getNamespace()) || !StringUtils.hasText(request.getExtension()) + || request.getRegistration() == null || request.getRegistration().isEmpty()) { + var json = TrustedPublisherJson + .error("The fields provider, namespace, extension and registration are mandatory."); + return new ResponseEntity<>(json, HttpStatus.BAD_REQUEST); + } + if (!Objects.equals(namespace, request.getNamespace())) { + var json = TrustedPublisherJson.error("The namespace in the path and in the request body must match."); + return new ResponseEntity<>(json, HttpStatus.BAD_REQUEST); + } + var publisher = trustedPublishing.registerTrustedPublisher( user, request.getNamespace(), diff --git a/server/src/main/java/org/eclipse/openvsx/trustedpublishing/TrustedPublishingConfig.java b/server/src/main/java/org/eclipse/openvsx/trustedpublishing/TrustedPublishingConfig.java index a068ba85e..f121c3a35 100644 --- a/server/src/main/java/org/eclipse/openvsx/trustedpublishing/TrustedPublishingConfig.java +++ b/server/src/main/java/org/eclipse/openvsx/trustedpublishing/TrustedPublishingConfig.java @@ -12,15 +12,57 @@ *****************************************************************************/ package org.eclipse.openvsx.trustedpublishing; +import java.net.URI; +import java.time.Duration; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import jakarta.annotation.PostConstruct; +import jakarta.validation.Valid; +import jakarta.validation.constraints.NotBlank; import org.jspecify.annotations.NonNull; +import org.jspecify.annotations.Nullable; import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.context.annotation.Configuration; +import org.springframework.validation.annotation.Validated; +import org.eclipse.openvsx.trustedpublishing.github.GitHubTrustedPublishingProvider; +import org.eclipse.openvsx.trustedpublishing.gitlab.GitLabTrustedPublishingProvider; + +/** + * The trusted publishing configuration. + *

+ * The scalar settings are read with {@code @Value}; the two that need a typed value - the token lifetime + * and the GitLab instance map - are bound by {@code @ConfigurationProperties}, whose binder converts them + * natively. Only fields with a setter are bound, so the two sets do not overlap. + *

+ * Every GitLab instance behaves the same way, differing only in id, name, URL and OIDC issuer, so instances + * are configured rather than coded. Only the public instance is configured out of the box; any other one - + * the Eclipse Foundation instance included - is added by configuration, and becomes usable once its id is + * listed in {@code ovsx.trusted-publishing.active-providers}: + * + *

+ * ovsx:
+ *   trusted-publishing:
+ *     active-providers: github,eclipse-gitlab
+ *     gitlab:
+ *       eclipse-gitlab:
+ *         name: Eclipse GitLab
+ *         url: https://gitlab.eclipse.org
+ *         issuer: https://gitlab.eclipse.org   # optional, defaults to the URL
+ * 
+ * + * The id is persisted with every registration, so renaming it hides the registrations made for it. + * Configuring the id of the default instance replaces it as a whole rather than patching single fields, + * so such an entry has to carry the name and the URL itself. + */ @Configuration +@ConfigurationProperties(prefix = "ovsx.trusted-publishing") +@Validated public class TrustedPublishingConfig { + /** * Whether trusted publishing is enabled at all. */ @@ -41,12 +83,27 @@ public class TrustedPublishingConfig { private List forbiddenJwtHeaders; /** - * The comma separated list of active trusted publishing providers. + * The comma separated list of active trusted publishing providers. An id listed here must be + * {@code github} or one of the configured GitLab instances. * Default: {@code github}. */ @Value("${ovsx.trusted-publishing.active-providers:github}") private List activeProviders; + /** + * How long an issued publishing token is valid. Must be positive: a token that does not expire is a + * long-lived credential, which is the very thing trusted publishing exists to avoid. The lifetime of + * ordinary personal access tokens is {@code ovsx.access-token.expiration} instead. + */ + private Duration tokenExpiration = Duration.ofMinutes(5); + + /** + * The known GitLab instances, keyed by provider id. Configured instances are added to the public + * instance, which stays available unless its id is redefined. + */ + @Valid + private Map gitlab = defaultGitLabInstances(); + public boolean isEnabled() { return enabled; } @@ -66,8 +123,47 @@ public List getActiveProviders() { return activeProviders; } + /** + * The configured GitLab instances, keyed by provider id. Whether an instance can actually be used + * is decided by {@link #getActiveProviders()}. + */ + @NonNull + public Map getGitlab() { + return gitlab; + } + + public void setGitlab(Map gitlab) { + this.gitlab = gitlab; + } + + /** + * How long an issued publishing token is valid, {@code ovsx.trusted-publishing.token-expiration}. + */ + @NonNull + public Duration getTokenExpiration() { + return tokenExpiration; + } + + public void setTokenExpiration(Duration tokenExpiration) { + this.tokenExpiration = tokenExpiration; + } + + private static Map defaultGitLabInstances() { + var instances = new LinkedHashMap(); + instances.put( + GitLabTrustedPublishingProvider.PROVIDER_ID, + new GitLabInstance("GitLab", GitLabTrustedPublishingProvider.PROVIDER_URL)); + return instances; + } + @PostConstruct public void validate() { + // checked whether or not the feature is on, so a typo does not lie in wait until it is turned on + var tokenExpiration = getTokenExpiration(); + if (tokenExpiration == null || !tokenExpiration.isPositive()) { + throw new IllegalStateException( + "ovsx.trusted-publishing.token-expiration must be a positive duration, got: " + tokenExpiration); + } if (enabled) { if (audience == null || audience.isBlank()) { throw new IllegalStateException("Trusted publishing is enabled, but audience is not configured"); @@ -80,6 +176,95 @@ public void validate() { throw new IllegalStateException( "Trusted publishing is enabled, but there are no active providers configured"); } + validateGitLabInstances(); + } + } + + private void validateGitLabInstances() { + for (var entry : getGitlab().entrySet()) { + var id = entry.getKey(); + var instance = entry.getValue(); + if (GitHubTrustedPublishingProvider.PROVIDER_ID.equals(id)) { + throw new IllegalStateException( + "GitLab instance '" + id + "' uses the provider id of the GitHub provider"); + } + // a configured instance replaces a default one as a whole, so it must carry every field itself + if (instance.getName() == null || instance.getName().isBlank() + || instance.getUrl() == null || instance.getUrl().isBlank()) { + throw new IllegalStateException("GitLab instance '" + id + "' has no name or no URL configured"); + } + for (var url : List.of(instance.getUrl(), instance.getIssuer())) { + if (hostOf(url) == null) { + throw new IllegalStateException("GitLab instance '" + id + "' has a malformed URL: " + url); + } + } + } + } + + @Nullable + private static String hostOf(String url) { + try { + return URI.create(url).getHost(); + } catch (IllegalArgumentException exc) { + return null; + } + } + + /** + * A single GitLab instance. + */ + public static class GitLabInstance { + + @NotBlank + private String name; + + @NotBlank + private String url; + + @Nullable + private String issuer; + + public GitLabInstance() { + // for configuration property binding + } + + public GitLabInstance(String name, String url) { + this.name = name; + this.url = url; + } + + /** + * The instance name, for human consumption. + */ + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + /** + * The base URL of the instance; the API and the {@code ci_config_ref_uri} claim are derived from it. + */ + public String getUrl() { + return url; + } + + public void setUrl(String url) { + this.url = url; + } + + /** + * The issuer to expect in the {@code iss} claim of issued OIDC ID tokens. GitLab issues tokens + * under its own base URL, so this defaults to {@link #getUrl()}. + */ + public String getIssuer() { + return issuer == null || issuer.isBlank() ? url : issuer; + } + + public void setIssuer(@Nullable String issuer) { + this.issuer = issuer; } } } diff --git a/server/src/main/java/org/eclipse/openvsx/trustedpublishing/TrustedPublishingService.java b/server/src/main/java/org/eclipse/openvsx/trustedpublishing/TrustedPublishingService.java index bcbbde234..106ff249f 100644 --- a/server/src/main/java/org/eclipse/openvsx/trustedpublishing/TrustedPublishingService.java +++ b/server/src/main/java/org/eclipse/openvsx/trustedpublishing/TrustedPublishingService.java @@ -14,11 +14,13 @@ import java.text.ParseException; import java.util.ArrayList; -import java.util.HashMap; +import java.util.Collections; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.stream.Collectors; import com.nimbusds.jwt.JWTParser; import jakarta.persistence.EntityManager; @@ -38,9 +40,7 @@ import org.eclipse.openvsx.json.ResultJson; import org.eclipse.openvsx.repositories.RepositoryService; import org.eclipse.openvsx.trustedpublishing.github.GitHubTrustedPublishingProvider; -import org.eclipse.openvsx.trustedpublishing.gitlab.EclipseGitLabTrustedPublishingProvider; import org.eclipse.openvsx.trustedpublishing.gitlab.GitLabTrustedPublishingProvider; -import org.eclipse.openvsx.trustedpublishing.gitlab.GitLabTrustedPublishingProviderSupport; import org.eclipse.openvsx.util.ErrorResultException; import org.eclipse.openvsx.util.NotFoundException; import org.eclipse.openvsx.util.TimeUtil; @@ -71,18 +71,47 @@ public TrustedPublishingService( this.entityManager = requireNonNull(entityManager); if (config.isEnabled()) { - this.providers = Map.of( - GitHubTrustedPublishingProvider.PROVIDER_ID, - new GitHubTrustedPublishingProvider(config), - GitLabTrustedPublishingProvider.PROVIDER_ID, - new GitLabTrustedPublishingProvider(config), - EclipseGitLabTrustedPublishingProvider.PROVIDER_ID, - new EclipseGitLabTrustedPublishingProvider(config)); + this.providers = createProviders(config); + warnAboutUnknownActiveProviders(config); } else { this.providers = Map.of(); } } + /** + * GitHub is a single, hard-wired provider; every configured GitLab instance becomes one of its own. + */ + private static Map createProviders(TrustedPublishingConfig config) { + // insertion-ordered, so the providers are always offered in the same order: GitHub first, then the + // GitLab instances as configured. An unordered map would reshuffle the list on every restart. + var providers = new LinkedHashMap(); + providers.put(GitHubTrustedPublishingProvider.PROVIDER_ID, new GitHubTrustedPublishingProvider(config)); + config.getGitlab() + .forEach( + (providerId, instance) -> providers.put( + providerId, + new GitLabTrustedPublishingProvider( + config, + providerId, + instance.getName(), + instance.getUrl(), + instance.getIssuer()))); + return Collections.unmodifiableMap(providers); + } + + /** + * An active provider without a matching definition is silently unusable, which is hard to tell apart + * from a working setup, so say so at startup. + */ + private void warnAboutUnknownActiveProviders(TrustedPublishingConfig config) { + var unknown = config.getActiveProviders().stream().filter(id -> !providers.containsKey(id)).toList(); + if (!unknown.isEmpty()) { + logger.warn( + "Trusted publishing lists active providers that are not configured and stay unusable: {}", + unknown); + } + } + public boolean isEnabled() { return config.isEnabled(); } @@ -217,15 +246,8 @@ public Map getTrustedPublisherProvider ensureEnabled(); return providers.entrySet().stream() .filter(e -> e.getValue().isActive()) - .collect(HashMap::new, (m, e) -> m.put(e.getKey(), e.getValue()), HashMap::putAll); - } - - /** - * Lists all trusted publisher providers. - */ - public Map getAllTrustedPublisherProviders() { - ensureEnabled(); - return providers; + .collect( + Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, (a, b) -> a, LinkedHashMap::new)); } /** @@ -289,7 +311,9 @@ public AccessTokenJson requestPublishToken(String namespaceName, String extensio // The issued token is TPT personal access token of the registering user scoped for selected namespace return tokens.createTrustedPublishingAccessToken( match, - TOKEN_DESCRIPTION_TEMPLATE.formatted(provider.getProviderId())); + TOKEN_DESCRIPTION_TEMPLATE.formatted(provider.getProviderId()), + config.getTokenExpiration(), + claims); } private Namespace requireOwnedNamespace(UserData user, String namespaceName) { diff --git a/server/src/main/java/org/eclipse/openvsx/trustedpublishing/gitlab/EclipseGitLabTrustedPublishingProvider.java b/server/src/main/java/org/eclipse/openvsx/trustedpublishing/gitlab/EclipseGitLabTrustedPublishingProvider.java deleted file mode 100644 index c24787fe5..000000000 --- a/server/src/main/java/org/eclipse/openvsx/trustedpublishing/gitlab/EclipseGitLabTrustedPublishingProvider.java +++ /dev/null @@ -1,28 +0,0 @@ -/****************************************************************************** - * Copyright (c) 2026 Contributors to the Eclipse Foundation. - * - * See the NOTICE file(s) distributed with this work for additional - * information regarding copyright ownership. - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Public License 2.0 which is available at - * https://www.eclipse.org/legal/epl-2.0. - * - * SPDX-License-Identifier: EPL-2.0 - *****************************************************************************/ -package org.eclipse.openvsx.trustedpublishing.gitlab; - -import org.eclipse.openvsx.trustedpublishing.TrustedPublishingConfig; - -/** - * GitLab provider for Eclipse GitLab Instance. - */ -public class EclipseGitLabTrustedPublishingProvider extends GitLabTrustedPublishingProviderSupport { - public static final String PROVIDER_ID = "eclipse-gitlab"; - public static final String PROVIDER_URL = "https://gitlab.eclipse.org"; - private static final String OIDC_ISSUER = "https://gitlab.eclipse.org"; - - public EclipseGitLabTrustedPublishingProvider(TrustedPublishingConfig config) { - super(config, PROVIDER_ID, "Eclipse GitLab", PROVIDER_URL, OIDC_ISSUER); - } -} diff --git a/server/src/main/java/org/eclipse/openvsx/trustedpublishing/gitlab/GitLabTrustedPublishingProvider.java b/server/src/main/java/org/eclipse/openvsx/trustedpublishing/gitlab/GitLabTrustedPublishingProvider.java index 1fee0449d..8092420e2 100644 --- a/server/src/main/java/org/eclipse/openvsx/trustedpublishing/gitlab/GitLabTrustedPublishingProvider.java +++ b/server/src/main/java/org/eclipse/openvsx/trustedpublishing/gitlab/GitLabTrustedPublishingProvider.java @@ -12,17 +12,167 @@ *****************************************************************************/ package org.eclipse.openvsx.trustedpublishing.gitlab; +import java.net.URI; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.jspecify.annotations.NonNull; +import org.springframework.core.ParameterizedTypeReference; +import org.springframework.http.MediaType; +import org.springframework.security.oauth2.jwt.Jwt; +import org.springframework.security.oauth2.jwt.JwtClaimNames; +import org.springframework.web.client.RestClientException; + +import org.eclipse.openvsx.json.TrustedPublisherInputJson; import org.eclipse.openvsx.trustedpublishing.TrustedPublishingConfig; +import org.eclipse.openvsx.trustedpublishing.TrustedPublishingProviderSupport; +import org.eclipse.openvsx.util.ErrorResultException; + +import static java.util.Objects.requireNonNull; /** - * GitLab provider for GitLab Public Instance. + * A GitLab instance as a trusted publishing provider. + *

+ * Every instance - the public one, the Eclipse Foundation one, any self-hosted one - is served by this + * class: they only differ in id, name, URL and OIDC issuer, which are configured through + * {@link org.eclipse.openvsx.trustedpublishing.TrustedPublishingConfig.GitLabInstance}. + * + * @see GitLab OpenID Connect */ -public class GitLabTrustedPublishingProvider extends GitLabTrustedPublishingProviderSupport { +public class GitLabTrustedPublishingProvider extends TrustedPublishingProviderSupport { + /** + * The provider id of the public GitLab instance, configured out of the box. + */ public static final String PROVIDER_ID = "gitlab"; + + /** + * The URL of the public GitLab instance. + */ public static final String PROVIDER_URL = "https://gitlab.com"; - private static final String OIDC_ISSUER = "https://gitlab.com"; - public GitLabTrustedPublishingProvider(TrustedPublishingConfig config) { - super(config, PROVIDER_ID, "GitLab", PROVIDER_URL, OIDC_ISSUER); + private static final String CLAIM_NAMESPACE_ID = "namespace_id"; // "72" + private static final String CLAIM_NAMESPACE_PATH = "namespace_path"; // "my-group" + private static final String CLAIM_PROJECT_ID = "project_id"; // "20" + private static final String CLAIM_PROJECT_PATH = "project_path"; // "my-group/my-project" + private static final String CLAIM_ENVIRONMENT = "environment"; // "prod"; optional + private static final String CLAIM_RUNNER_ENVIRONMENT = "runner_environment"; // "gitlab-hosted" + private static final String CLAIM_CI_CONFIG_REF_URI = "ci_config_ref_uri"; // "gitlab.example.com/my-group/my-project//.gitlab-ci.yml@refs/heads/main" + + private static final String API_RESOLVE_REQUEST = "/api/v4/projects/{path}"; + + private static final String REG_NAMESPACE = "namespace"; + private static final String REG_PROJECT = "project"; + private static final String REG_WORKFLOW = "workflow"; + private static final String REG_ENVIRONMENT = "environment"; + private static final List REGISTRATION_INPUTS = List.of( + TrustedPublisherInputJson.create(REG_NAMESPACE, "Namespace", false), + TrustedPublisherInputJson.create(REG_PROJECT, "Project name", false), + TrustedPublisherInputJson.create(REG_WORKFLOW, "Top-level CI filename", false), + TrustedPublisherInputJson.create(REG_ENVIRONMENT, "Environment name (optional)", true)); + + /** + * The instance part of the {@code ci_config_ref_uri} claim: the host of the instance, plus the path + * when the instance is served under a relative URL root. + */ + private final String instanceLocation; + + public GitLabTrustedPublishingProvider( + TrustedPublishingConfig config, + String providerId, + String providerName, + String providerUrl, + String oidcIssuer + ) { + super(config, providerId, providerName, providerUrl, oidcIssuer, REGISTRATION_INPUTS); + this.instanceLocation = instanceLocation(providerUrl); + } + + private static String instanceLocation(String providerUrl) { + URI uri = URI.create(providerUrl); + String host = uri.getHost(); + if (host == null) { + throw new IllegalArgumentException("Not a valid GitLab instance URL: " + providerUrl); + } + String path = uri.getPath(); + while (path != null && path.endsWith("/")) { + path = path.substring(0, path.length() - 1); + } + return path == null || path.isEmpty() ? host : host + path; + } + + @NonNull + @Override + protected Map extractClaims(Jwt jwt) { + requireNonNull(jwt); + HashMap result = new HashMap<>(7); + mustClaim(jwt, JwtClaimNames.SUB, result); + mustClaim(jwt, CLAIM_NAMESPACE_ID, result); + mustClaim(jwt, CLAIM_NAMESPACE_PATH, result); + mustClaim(jwt, CLAIM_PROJECT_ID, result); + mustClaim(jwt, CLAIM_PROJECT_PATH, result); + mayClaim(jwt, CLAIM_ENVIRONMENT, result); + mustClaim(jwt, CLAIM_RUNNER_ENVIRONMENT, result); + mustClaim(jwt, CLAIM_CI_CONFIG_REF_URI, result); + return result; + } + + @NonNull + @Override + protected Map extractRequest(Map registration) throws ErrorResultException { + requireNonNull(registration); + + final String namespace = mustRegister(registration, REG_NAMESPACE); + final String project = mustRegister(registration, REG_PROJECT); + final String workflow = mustRegister(registration, REG_WORKFLOW); + final String environment = registration.get(REG_ENVIRONMENT); + final String projectPath = namespace + "/" + project; + + Map response = resolve(projectPath); + if (response == null || !(response.get("id") instanceof Number projectId) + || !(response.get("namespace") instanceof Map namespaceMap) + || !(namespaceMap.get("id") instanceof Number namespaceId)) { + throw new ErrorResultException("Unexpected GitLab response for project " + projectPath); + } + + HashMap result = new HashMap<>(); + result.put(CLAIM_NAMESPACE_ID, String.valueOf(namespaceId.longValue())); + result.put(CLAIM_NAMESPACE_PATH, namespace); + result.put(CLAIM_PROJECT_ID, String.valueOf(projectId.longValue())); + result.put(CLAIM_PROJECT_PATH, projectPath); + // registered without the "@" part: publishing is trusted regardless of branch or tag + result.put(CLAIM_CI_CONFIG_REF_URI, instanceLocation + "/" + projectPath + "//" + workflow); + if (environment != null) { + result.put(CLAIM_ENVIRONMENT, environment); + } + return result; + } + + /** + * Pulled out for testability; is mocked in UT to prevent real remote access. + */ + protected Map resolve(String projectPath) throws ErrorResultException { + try { + // the {path} template variable is URL-encoded by RestClient, turning "/" into "%2F" as GitLab expects + return restClient.get() + .uri(providerUrl + API_RESOLVE_REQUEST, projectPath) + .accept(MediaType.APPLICATION_JSON) + .retrieve() + .body(new ParameterizedTypeReference<>() { + }); + } catch (RestClientException e) { + throw new ErrorResultException("Could not resolve GitLab project " + projectPath, e); + } + } + + @Override + public boolean matches(@NonNull Map registered, @NonNull Map token) { + requireNonNull(registered); + requireNonNull(token); + return claimEquals(CLAIM_PROJECT_ID, registered, token) + && claimEquals(CLAIM_NAMESPACE_ID, registered, token) + && registered.get(CLAIM_CI_CONFIG_REF_URI) != null + && registered.get(CLAIM_CI_CONFIG_REF_URI).equals(stripRef(token.get(CLAIM_CI_CONFIG_REF_URI))) + && pinnedClaimMatches(CLAIM_ENVIRONMENT, registered, token); } } diff --git a/server/src/main/java/org/eclipse/openvsx/trustedpublishing/gitlab/GitLabTrustedPublishingProviderSupport.java b/server/src/main/java/org/eclipse/openvsx/trustedpublishing/gitlab/GitLabTrustedPublishingProviderSupport.java deleted file mode 100644 index 930b05dac..000000000 --- a/server/src/main/java/org/eclipse/openvsx/trustedpublishing/gitlab/GitLabTrustedPublishingProviderSupport.java +++ /dev/null @@ -1,147 +0,0 @@ -/****************************************************************************** - * Copyright (c) 2026 Contributors to the Eclipse Foundation. - * - * See the NOTICE file(s) distributed with this work for additional - * information regarding copyright ownership. - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Public License 2.0 which is available at - * https://www.eclipse.org/legal/epl-2.0. - * - * SPDX-License-Identifier: EPL-2.0 - *****************************************************************************/ -package org.eclipse.openvsx.trustedpublishing.gitlab; - -import java.net.URI; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -import org.jspecify.annotations.NonNull; -import org.springframework.core.ParameterizedTypeReference; -import org.springframework.http.MediaType; -import org.springframework.security.oauth2.jwt.Jwt; -import org.springframework.security.oauth2.jwt.JwtClaimNames; -import org.springframework.web.client.RestClientException; - -import org.eclipse.openvsx.json.TrustedPublisherInputJson; -import org.eclipse.openvsx.trustedpublishing.TrustedPublishingConfig; -import org.eclipse.openvsx.trustedpublishing.TrustedPublishingProviderSupport; -import org.eclipse.openvsx.util.ErrorResultException; - -import static java.util.Objects.requireNonNull; - -/** - * GitLab specific support. - * - * @see GitLab OpenID Connect - */ -public abstract class GitLabTrustedPublishingProviderSupport extends TrustedPublishingProviderSupport { - private static final String CLAIM_NAMESPACE_ID = "namespace_id"; // "72" - private static final String CLAIM_NAMESPACE_PATH = "namespace_path"; // "my-group" - private static final String CLAIM_PROJECT_ID = "project_id"; // "20" - private static final String CLAIM_PROJECT_PATH = "project_path"; // "my-group/my-project" - private static final String CLAIM_ENVIRONMENT = "environment"; // "prod"; optional - private static final String CLAIM_RUNNER_ENVIRONMENT = "runner_environment"; // "gitlab-hosted" - private static final String CLAIM_CI_CONFIG_REF_URI = "ci_config_ref_uri"; // "gitlab.example.com/my-group/my-project//.gitlab-ci.yml@refs/heads/main" - - private static final String API_RESOLVE_REQUEST = "/api/v4/projects/{path}"; - - private static final String REG_NAMESPACE = "namespace"; - private static final String REG_PROJECT = "project"; - private static final String REG_WORKFLOW = "workflow"; - private static final String REG_ENVIRONMENT = "environment"; - private static final List REGISTRATION_INPUTS = List.of( - TrustedPublisherInputJson.create(REG_NAMESPACE, "Namespace", false), - TrustedPublisherInputJson.create(REG_PROJECT, "Project name", false), - TrustedPublisherInputJson.create(REG_WORKFLOW, "Top-level CI filename", false), - TrustedPublisherInputJson.create(REG_ENVIRONMENT, "Environment name (optional)", true)); - - protected GitLabTrustedPublishingProviderSupport( - TrustedPublishingConfig config, - String providerId, - String providerName, - String providerUrl, - String oidcIssuer - ) { - super(config, providerId, providerName, providerUrl, oidcIssuer, REGISTRATION_INPUTS); - } - - @NonNull - @Override - protected Map extractClaims(Jwt jwt) { - requireNonNull(jwt); - HashMap result = new HashMap<>(7); - mustClaim(jwt, JwtClaimNames.SUB, result); - mustClaim(jwt, CLAIM_NAMESPACE_ID, result); - mustClaim(jwt, CLAIM_NAMESPACE_PATH, result); - mustClaim(jwt, CLAIM_PROJECT_ID, result); - mustClaim(jwt, CLAIM_PROJECT_PATH, result); - mayClaim(jwt, CLAIM_ENVIRONMENT, result); - mustClaim(jwt, CLAIM_RUNNER_ENVIRONMENT, result); - mustClaim(jwt, CLAIM_CI_CONFIG_REF_URI, result); - return result; - } - - @NonNull - @Override - protected Map extractRequest(Map registration) throws ErrorResultException { - requireNonNull(registration); - - final String namespace = mustRegister(registration, REG_NAMESPACE); - final String project = mustRegister(registration, REG_PROJECT); - final String workflow = mustRegister(registration, REG_WORKFLOW); - final String environment = registration.get(REG_ENVIRONMENT); - final String projectPath = namespace + "/" + project; - - Map response = resolve(projectPath); - if (response == null || !(response.get("id") instanceof Number projectId) - || !(response.get("namespace") instanceof Map namespaceMap) - || !(namespaceMap.get("id") instanceof Number namespaceId)) { - throw new ErrorResultException("Unexpected GitLab response for project " + projectPath); - } - - HashMap result = new HashMap<>(); - result.put(CLAIM_NAMESPACE_ID, String.valueOf(namespaceId.longValue())); - result.put(CLAIM_NAMESPACE_PATH, namespace); - result.put(CLAIM_PROJECT_ID, String.valueOf(projectId.longValue())); - result.put(CLAIM_PROJECT_PATH, projectPath); - // registered without the "@" part: publishing is trusted regardless of branch or tag - result.put( - CLAIM_CI_CONFIG_REF_URI, - URI.create(providerUrl).getHost() + "/" + projectPath - + "//" + workflow); - if (environment != null) { - result.put(CLAIM_ENVIRONMENT, environment); - } - return result; - } - - /** - * Pulled out for testability; is mocked in UT to prevent real remote access. - */ - protected Map resolve(String projectPath) throws ErrorResultException { - try { - // the {path} template variable is URL-encoded by RestClient, turning "/" into "%2F" as GitLab expects - return restClient.get() - .uri(providerUrl + API_RESOLVE_REQUEST, projectPath) - .accept(MediaType.APPLICATION_JSON) - .retrieve() - .body(new ParameterizedTypeReference<>() { - }); - } catch (RestClientException e) { - throw new ErrorResultException("Could not resolve GitLab project " + projectPath, e); - } - } - - @Override - public boolean matches(@NonNull Map registered, @NonNull Map token) { - requireNonNull(registered); - requireNonNull(token); - return claimEquals(CLAIM_PROJECT_ID, registered, token) - && claimEquals(CLAIM_NAMESPACE_ID, registered, token) - && registered.get(CLAIM_CI_CONFIG_REF_URI) != null - && registered.get(CLAIM_CI_CONFIG_REF_URI).equals(stripRef(token.get(CLAIM_CI_CONFIG_REF_URI))) - && pinnedClaimMatches(CLAIM_ENVIRONMENT, registered, token); - } -} diff --git a/server/src/main/java/org/eclipse/openvsx/util/auth/AccessTokenAuthentication.java b/server/src/main/java/org/eclipse/openvsx/util/auth/AccessTokenAuthentication.java index 9728fc708..1ace37eaa 100644 --- a/server/src/main/java/org/eclipse/openvsx/util/auth/AccessTokenAuthentication.java +++ b/server/src/main/java/org/eclipse/openvsx/util/auth/AccessTokenAuthentication.java @@ -12,15 +12,29 @@ *****************************************************************************/ package org.eclipse.openvsx.util.auth; +import java.util.Map; + import org.jspecify.annotations.NonNull; +import org.jspecify.annotations.Nullable; import org.eclipse.openvsx.entities.PersonalAccessTokenType; import org.eclipse.openvsx.entities.UserData; /** * Represents user who presented a valid access token. + * + * @param tokenId the token that authenticated this request, recorded on whatever it publishes as + * best-effort provenance. Kept as an id rather than the entity: nothing here should hold + * a credential open, and the reference is allowed to decay when the token row goes. + * @param claims for a trusted publishing token, the OIDC identity the provider asserted at the exchange, + * which the publish copies onto the version. Null for any other kind of token. */ -public record AccessTokenAuthentication(UserData userData, PersonalAccessTokenType type) implements AuthenticatedUser { +public record AccessTokenAuthentication( + UserData userData, + PersonalAccessTokenType type, + long tokenId, + @Nullable Map claims +) implements AuthenticatedUser { @Override public @NonNull AuthenticationType authenticationType() { return AuthenticationType.TOKEN; diff --git a/server/src/main/resources/db/migration/V1_72__Trusted_Publisher.sql b/server/src/main/resources/db/migration/V1_72__Trusted_Publisher.sql index 7c8d2f079..315b06b61 100644 --- a/server/src/main/resources/db/migration/V1_72__Trusted_Publisher.sql +++ b/server/src/main/resources/db/migration/V1_72__Trusted_Publisher.sql @@ -5,11 +5,18 @@ CREATE SEQUENCE IF NOT EXISTS trusted_publisher_seq START WITH 1 INCREMENT BY 1; CREATE TABLE IF NOT EXISTS public.trusted_publisher ( id BIGINT NOT NULL PRIMARY KEY DEFAULT nextval('trusted_publisher_seq'), - extension_id BIGINT NOT NULL REFERENCES public.extension(id), + -- A registration only means anything for the extension it points at, so it goes when that extension + -- is purged. Cascading rather than restricting, because purging is an administrative action that has + -- to succeed: without this, an extension that ever had a trusted publisher could never be purged. + extension_id BIGINT NOT NULL REFERENCES public.extension(id) ON DELETE CASCADE, provider CHARACTER VARYING(32) NOT NULL, registration JSONB NOT NULL, claims JSONB NOT NULL, - created_by BIGINT NOT NULL REFERENCES public.user_data(id), + -- Only a namespace owner may register a trusted publisher, so a registration cannot outlive its + -- author any more than it outlives their ownership. Deleting a user already revokes their + -- registrations by way of their memberships; this is the constraint saying the same thing, and keeps + -- this reference consistent with the others in this migration. + created_by BIGINT NOT NULL REFERENCES public.user_data(id) ON DELETE CASCADE, created_timestamp TIMESTAMP without time zone NOT NULL ); @@ -24,12 +31,21 @@ ALTER TABLE ONLY public.personal_access_token ADD COLUMN IF NOT EXISTS version SMALLINT, -- the type column LLT, OTT or TPT ADD COLUMN IF NOT EXISTS type CHARACTER VARYING(32), - -- optional; the extension that the token is scoped to - ADD COLUMN IF NOT EXISTS scope_extension_id BIGINT REFERENCES public.extension(id), - -- optional; the namespace that the token is scoped to - ADD COLUMN IF NOT EXISTS scope_namespace_id BIGINT REFERENCES public.namespace(id), - -- optional; the trusted publisher that the token was created for (token must remain; registration may be deleted) - ADD COLUMN IF NOT EXISTS trusted_publisher_id BIGINT REFERENCES public.trusted_publisher(id) ON DELETE SET NULL; + -- optional; the extension that the token is scoped to. Deleted with it rather than detached from + -- it: AccessTokenService.getScope treats a token with neither scope set as unrestricted, so setting + -- this to NULL would silently widen a scoped token instead of retiring it. A token scoped to an + -- extension that no longer exists can authorize nothing anyway. + ADD COLUMN IF NOT EXISTS scope_extension_id BIGINT REFERENCES public.extension(id) ON DELETE CASCADE, + -- optional; the namespace that the token is scoped to. Cascaded for the same reason. + ADD COLUMN IF NOT EXISTS scope_namespace_id BIGINT REFERENCES public.namespace(id) ON DELETE CASCADE, + -- optional; the trusted publisher that the token was created for. Deleting a registration retires the + -- tokens issued under it: they may only ever publish the one extension it was made for, so once it is + -- gone they can authorize nothing. + ADD COLUMN IF NOT EXISTS trusted_publisher_id BIGINT REFERENCES public.trusted_publisher(id) ON DELETE CASCADE, + -- optional; the OIDC claims the token was exchanged for, carried from the exchange to the publish that + -- uses it. This row does not keep them - a one-time token is deleted as it is used - it hands them to + -- the version being published, see extension_version.published_provenance below. + ADD COLUMN IF NOT EXISTS claims JSONB; -- set OTT based on description (is hardwired in codebase) UPDATE public.personal_access_token @@ -58,7 +74,14 @@ ALTER TABLE ONLY public.personal_access_token -- Every read path that asks "who published this?" uses this column from now on. ALTER TABLE public.extension_version ADD COLUMN published_by_id BIGINT, - ADD COLUMN published_with_tt CHARACTER VARYING(32); + ADD COLUMN published_with_tt CHARACTER VARYING(32), + -- The OIDC identity that produced this version, for versions published through trusted publishing: + -- the immutable repository and owner ids and the workflow reference including the ref it ran on, as + -- the provider asserted them at the exchange. Copied rather than referenced for the usual reason - + -- the token is deleted as it is used, and the registration can be revoked - and this link from a + -- published artifact back to the workflow run is most of what trusted publishing buys over a token + -- somebody pasted into CI. + ADD COLUMN published_provenance JSONB; -- Backfill from the only place the answer exists today. Rows whose published_with_id is already NULL -- have nothing to derive it from and stay NULL, which is why this column is deliberately left nullable: @@ -78,13 +101,17 @@ ALTER TABLE public.extension_version CREATE INDEX extension_version__published_by_id__idx ON public.extension_version (published_by_id); -- published_with_id keeps recording which credential was used, but as best-effort provenance rather --- than a hard dependency: deleting a token in future clears the reference instead of being refused by --- the database. The base migration left this constraint named by Hibernate; drop it by that generated --- name and recreate under the naming convention used everywhere else on this table. +-- than a hard dependency: it answers "what did this token publish" after a leak, while authorship is +-- recorded permanently in published_by_id above. Deleting a token clears the reference instead of being +-- refused by the database, so a one-time token used up, or a forgotten user's tokens, simply leave it +-- null. The base migration left this constraint named by Hibernate; drop it by that generated name and +-- recreate under the naming convention used everywhere else on this table. ALTER TABLE public.extension_version DROP CONSTRAINT fk70khj8pm0vacasuiiaq0w0r80; ALTER TABLE public.extension_version - DROP COLUMN published_with_id; + ADD CONSTRAINT extension_version_published_with_id_fkey + FOREIGN KEY (published_with_id) REFERENCES public.personal_access_token(id) ON DELETE SET NULL; + -- and now we can delete all inactive one-time-usable personal access tokens DELETE FROM public.personal_access_token pat diff --git a/server/src/test/java/org/eclipse/openvsx/ExtensionServiceTest.java b/server/src/test/java/org/eclipse/openvsx/ExtensionServiceTest.java index e84891c0d..02d6f536c 100644 --- a/server/src/test/java/org/eclipse/openvsx/ExtensionServiceTest.java +++ b/server/src/test/java/org/eclipse/openvsx/ExtensionServiceTest.java @@ -318,7 +318,9 @@ void shouldNotScanWhenPublishPreconditionsFail() { var content = new ByteArrayInputStream("extension package".getBytes(StandardCharsets.UTF_8)); assertThatThrownBy( - () -> svc.publishVersion(content, new AccessTokenAuthentication(token.getUser(), token.getType()))) + () -> svc.publishVersion( + content, + new AccessTokenAuthentication(token.getUser(), token.getType(), token.getId(), null))) .isInstanceOf(ErrorResultException.class) .hasMessageContaining("Insufficient access rights"); @@ -344,7 +346,9 @@ void shouldRejectAPackageExceedingTheMaxContentSize() { var content = new DrainOnCloseInputStream(raw, maxContentSize); assertThatThrownBy( - () -> svc.publishVersion(content, new AccessTokenAuthentication(token.getUser(), token.getType()))) + () -> svc.publishVersion( + content, + new AccessTokenAuthentication(token.getUser(), token.getType(), token.getId(), null))) .isInstanceOf(ErrorResultException.class) .hasMessageContaining("exceeds the size limit") .extracting(exc -> ((ErrorResultException) exc).getStatus()) diff --git a/server/src/test/java/org/eclipse/openvsx/LocalRegistryServiceTest.java b/server/src/test/java/org/eclipse/openvsx/LocalRegistryServiceTest.java index 995edafdb..6044c5835 100644 --- a/server/src/test/java/org/eclipse/openvsx/LocalRegistryServiceTest.java +++ b/server/src/test/java/org/eclipse/openvsx/LocalRegistryServiceTest.java @@ -164,7 +164,7 @@ void shouldNotDeleteTempFileOnceOwnershipIsHandedToAsyncPublish() throws IOExcep extVersion.setExtension(extension); extVersion.setVersion("1.0.0"); - var tau = new AccessTokenAuthentication(token.getUser(), token.getType()); + var tau = new AccessTokenAuthentication(token.getUser(), token.getType(), token.getId(), null); when(extensions.createExtensionFile(any())).thenReturn(tempFile); when(tokens.useAccessToken(eq("tok"), any())).thenReturn(tau); diff --git a/server/src/test/java/org/eclipse/openvsx/RegistryAPITest.java b/server/src/test/java/org/eclipse/openvsx/RegistryAPITest.java index 85fc0d516..c52f48e3f 100644 --- a/server/src/test/java/org/eclipse/openvsx/RegistryAPITest.java +++ b/server/src/test/java/org/eclipse/openvsx/RegistryAPITest.java @@ -27,6 +27,7 @@ import jakarta.persistence.EntityManager; import org.apache.commons.lang3.ArrayUtils; import org.jobrunr.scheduling.JobRequestScheduler; +import org.jooq.DSLContext; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -109,6 +110,7 @@ @WebMvcTest(RegistryAPI.class) @MockitoBean( types = { + DSLContext.class, ClientRegistrationRepository.class, UpstreamRegistryService.class, GoogleCloudStorageService.class, @@ -3675,12 +3677,12 @@ AccessTokenConfig tokenConfig() { @Bean AccessTokenService tokenService( AccessTokenConfig config, - UUIDService uuidService, EntityManager entityManager, RepositoryService repositories, - MailService mailService + MailService mailService, + DSLContext dsl ) { - return new AccessTokenService(config, uuidService, entityManager, repositories, mailService); + return new AccessTokenService(config, entityManager, repositories, mailService, dsl); } @Bean diff --git a/server/src/test/java/org/eclipse/openvsx/UserAPITest.java b/server/src/test/java/org/eclipse/openvsx/UserAPITest.java index 67c351315..c9eb21de1 100644 --- a/server/src/test/java/org/eclipse/openvsx/UserAPITest.java +++ b/server/src/test/java/org/eclipse/openvsx/UserAPITest.java @@ -19,6 +19,8 @@ import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import jakarta.persistence.EntityManager; import org.jobrunr.scheduling.JobRequestScheduler; +import org.jooq.DSLContext; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mockito; import org.mockito.invocation.InvocationOnMock; @@ -83,6 +85,7 @@ @WebMvcTest(UserAPI.class) @MockitoBean( types = { + DSLContext.class, EclipseService.class, ClientRegistrationRepository.class, StorageUtilService.class, @@ -102,6 +105,14 @@ ) class UserAPITest { + @BeforeEach + void noTrustedPublishersByDefault() { + // the real repository hands back an empty Streamable rather than null; only the trusted + // publishing tests care what it actually holds + Mockito.when(repositories.findTrustedPublishersByNamespaceAndCreatedBy(any(), any())) + .thenReturn(Streamable.empty()); + } + @MockitoSpyBean UserService users; @@ -175,7 +186,7 @@ void testAccessTokensNotLoggedIn() throws Exception { @Test void testCreateAccessToken() throws Exception { mockUserData(); - Mockito.doReturn("foobar").when(accessTokenService).generateTokenValue(); + Mockito.doReturn("foobar").when(accessTokenService).generateTokenValue(any()); mockMvc.perform( post("/user/token/create?description={description}", "This is my token") .with(user("test_user")) @@ -1178,12 +1189,12 @@ AccessTokenConfig tokenConfig() { @Bean AccessTokenService accessTokenService( AccessTokenConfig config, - UUIDService uuidService, EntityManager entityManager, RepositoryService repositories, - MailService mailService + MailService mailService, + DSLContext dsl ) { - return new AccessTokenService(config, uuidService, entityManager, repositories, mailService); + return new AccessTokenService(config, entityManager, repositories, mailService, dsl); } @Bean diff --git a/server/src/test/java/org/eclipse/openvsx/UserServiceTest.java b/server/src/test/java/org/eclipse/openvsx/UserServiceTest.java index f48f6254c..8c6764304 100644 --- a/server/src/test/java/org/eclipse/openvsx/UserServiceTest.java +++ b/server/src/test/java/org/eclipse/openvsx/UserServiceTest.java @@ -21,13 +21,17 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.TestConfiguration; import org.springframework.context.annotation.Bean; +import org.springframework.data.util.Streamable; import org.springframework.security.authentication.AuthenticationServiceException; import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository; import org.springframework.test.context.bean.override.mockito.MockitoBean; import org.springframework.test.context.junit.jupiter.SpringExtension; import org.eclipse.openvsx.cache.CacheService; +import org.eclipse.openvsx.entities.Extension; import org.eclipse.openvsx.entities.Namespace; +import org.eclipse.openvsx.entities.NamespaceMembership; +import org.eclipse.openvsx.entities.TrustedPublisher; import org.eclipse.openvsx.entities.UserData; import org.eclipse.openvsx.json.NamespaceDetailsJson; import org.eclipse.openvsx.repositories.RepositoryService; @@ -155,6 +159,97 @@ void shouldSkipCollisionCheckWhenDisplayNameUnchanged() { verify(repositories, never()).findConflictingNamespaces(anyString(), any(Namespace.class)); } + // A trusted publisher may only be registered by a namespace owner, so it must not survive the + // ownership it was created under - the publishing tokens issued under it would outlive it otherwise. + + @Test + void shouldDeleteTrustedPublishersWhenAnOwnerLeavesTheNamespace() { + var user = mockUser("auth"); + var namespace = mockNamespace(); + var membership = mockMembership(user, namespace, NamespaceMembership.ROLE_OWNER); + var trustedPublisher = mockTrustedPublisher(namespace); + Mockito.when(repositories.findMembership(user, namespace)).thenReturn(membership); + Mockito.when(repositories.findTrustedPublishersByNamespaceAndCreatedBy(namespace, user)) + .thenReturn(Streamable.of(trustedPublisher)); + + users.removeNamespaceMember(namespace, user); + + verify(repositories).deleteTrustedPublisher(trustedPublisher); + } + + @Test + void shouldDeleteTrustedPublishersWhenAnOwnerIsDemotedToContributor() { + var user = mockUser("auth"); + var namespace = mockNamespace(); + var membership = mockMembership(user, namespace, NamespaceMembership.ROLE_OWNER); + var trustedPublisher = mockTrustedPublisher(namespace); + Mockito.when(repositories.findMembership(user, namespace)).thenReturn(membership); + Mockito.when(repositories.findTrustedPublishersByNamespaceAndCreatedBy(namespace, user)) + .thenReturn(Streamable.of(trustedPublisher)); + + users.addNamespaceMember(namespace, user, NamespaceMembership.ROLE_CONTRIBUTOR); + + assertEquals(NamespaceMembership.ROLE_CONTRIBUTOR, membership.getRole()); + verify(repositories).deleteTrustedPublisher(trustedPublisher); + } + + @Test + void shouldKeepTrustedPublishersWhenAContributorIsPromotedToOwner() { + var user = mockUser("auth"); + var namespace = mockNamespace(); + var membership = mockMembership(user, namespace, NamespaceMembership.ROLE_CONTRIBUTOR); + Mockito.when(repositories.findMembership(user, namespace)).thenReturn(membership); + + users.addNamespaceMember(namespace, user, NamespaceMembership.ROLE_OWNER); + + assertEquals(NamespaceMembership.ROLE_OWNER, membership.getRole()); + verify(repositories, never()).findTrustedPublishersByNamespaceAndCreatedBy(any(), any()); + verify(repositories, never()).deleteTrustedPublisher(any()); + } + + @Test + void shouldKeepTrustedPublishersOfEveryOtherOwner() { + var leaving = mockUser(1, "leaving_user", "auth-1"); + var namespace = mockNamespace(); + var membership = mockMembership(leaving, namespace, NamespaceMembership.ROLE_OWNER); + Mockito.when(repositories.findMembership(leaving, namespace)).thenReturn(membership); + // only the registrations this user created are looked up, so the other owners' ones stay + Mockito.when(repositories.findTrustedPublishersByNamespaceAndCreatedBy(namespace, leaving)) + .thenReturn(Streamable.empty()); + + users.removeNamespaceMember(namespace, leaving); + + verify(repositories, never()).deleteTrustedPublisher(any()); + } + + private Namespace mockNamespace() { + var namespace = new Namespace(); + namespace.setId(1); + namespace.setName("my-ns"); + return namespace; + } + + private NamespaceMembership mockMembership(UserData user, Namespace namespace, String role) { + var membership = new NamespaceMembership(); + membership.setUser(user); + membership.setNamespace(namespace); + membership.setRole(role); + return membership; + } + + private TrustedPublisher mockTrustedPublisher(Namespace namespace) { + var extension = new Extension(); + extension.setId(2); + extension.setName("my-ext"); + extension.setNamespace(namespace); + + var trustedPublisher = new TrustedPublisher(); + trustedPublisher.setId(3); + trustedPublisher.setExtension(extension); + trustedPublisher.setProvider("github"); + return trustedPublisher; + } + private UserData mockUser(String authId) { return mockUser(1, "test_user", authId); } diff --git a/server/src/test/java/org/eclipse/openvsx/accesstoken/AccessTokenConcurrentWriteTest.java b/server/src/test/java/org/eclipse/openvsx/accesstoken/AccessTokenConcurrentWriteTest.java new file mode 100644 index 000000000..c7c4863ca --- /dev/null +++ b/server/src/test/java/org/eclipse/openvsx/accesstoken/AccessTokenConcurrentWriteTest.java @@ -0,0 +1,284 @@ +/****************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + *****************************************************************************/ +package org.eclipse.openvsx.accesstoken; + +import java.time.LocalDateTime; +import java.util.Map; + +import jakarta.persistence.EntityManager; +import org.jobrunr.scheduling.JobRequestScheduler; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.bean.override.mockito.MockitoBean; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.TransactionDefinition; +import org.springframework.transaction.support.TransactionTemplate; + +import org.eclipse.openvsx.AbstractPostgresContainerTest; +import org.eclipse.openvsx.entities.Extension; +import org.eclipse.openvsx.entities.ExtensionVersion; +import org.eclipse.openvsx.entities.Namespace; +import org.eclipse.openvsx.entities.PersonalAccessToken; +import org.eclipse.openvsx.entities.PersonalAccessTokenType; +import org.eclipse.openvsx.entities.UserData; +import org.eclipse.openvsx.search.SearchUtilService; +import org.eclipse.openvsx.util.TargetPlatform; +import org.eclipse.openvsx.util.TimeUtil; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Token rows against a real database: what a write actually sends, and what expiry actually leaves behind. + * Neither is visible without one - the first is decided by Hibernate's generated UPDATE, the second by a + * native query. + */ +@SpringBootTest +class AccessTokenConcurrentWriteTest extends AbstractPostgresContainerTest { + + @Autowired + AccessTokenService accessTokens; + + @Autowired + EntityManager em; + + @Autowired + PlatformTransactionManager txManager; + + @MockitoBean + SearchUtilService search; + + @MockitoBean + JobRequestScheduler scheduler; + + @Test + void upgradingATokenDoesNotResurrectOneRevokedMeanwhile() { + var tokenId = persistLegacyToken(); + + new TransactionTemplate(txManager).executeWithoutResult(status -> { + // what the upgrade job holds: a snapshot taken before the revoke below, so its copy still + // says active = true + var token = em.find(PersonalAccessToken.class, tokenId); + assertThat(token.isActive()).isTrue(); + + revokeInAnotherTransaction(tokenId); + + // and now it writes the two columns it actually cares about + token.setValue("hashed-" + token.getValue()); + token.setVersion(1); + }); + + new TransactionTemplate(txManager).executeWithoutResult(status -> { + var token = em.find(PersonalAccessToken.class, tokenId); + assertThat(token.getVersion()).isEqualTo(1); + assertThat(token.getValue()).startsWith("hashed-"); + // the revoke has to survive: without @DynamicUpdate the upgrade's full-row UPDATE writes + // active = true back over it, handing a revoked token back to its holder + assertThat(token.isActive()).isFalse(); + }); + + cleanUp(tokenId); + } + + // Using a one-time token deletes it, so keeping the ones that expired unused would retain exactly the + // rows nobody has a question about - and one is minted per trusted publishing exchange. + @Test + void expiryDeletesAOneTimeTokenAndOnlyDeactivatesALongLivedOne() { + var expired = TimeUtil.getCurrentUTC().minusMinutes(1); + var oneTime = persistToken("expiry-tpt", PersonalAccessTokenType.TPT, 1, expired); + var longLived = persistToken("expiry-llt", PersonalAccessTokenType.LLT, 1, expired); + + new TransactionTemplate(txManager).executeWithoutResult(status -> accessTokens.expireAccessTokens()); + + new TransactionTemplate(txManager).executeWithoutResult(status -> { + assertThat(em.find(PersonalAccessToken.class, oneTime)).isNull(); + var kept = em.find(PersonalAccessToken.class, longLived); + // a user is shown their own expired tokens, and the notification mails read them + assertThat(kept).isNotNull(); + assertThat(kept.isActive()).isFalse(); + }); + + cleanUp(longLived); + // the one-time token's own row is gone, but its user is not + cleanUpUser("expiry-tpt-user"); + } + + // Best-effort provenance: it answers "what did this credential publish" after a leak, and must give + // way rather than stand in the way when the token itself is deleted. + @Test + void aVersionRemembersTheTokenItWasPublishedWithUntilThatTokenIsDeleted() { + var tokenId = persistToken("provenance", PersonalAccessTokenType.LLT, 1, null); + var versionId = persistVersionPublishedWith(tokenId); + + new TransactionTemplate(txManager).executeWithoutResult(status -> { + assertThat(em.find(ExtensionVersion.class, versionId).getPublishedWithId()).isEqualTo(tokenId); + }); + + new TransactionTemplate(txManager) + .executeWithoutResult(status -> em.remove(em.find(PersonalAccessToken.class, tokenId))); + + new TransactionTemplate(txManager).executeWithoutResult(status -> { + var version = em.find(ExtensionVersion.class, versionId); + // the reference decays, the authorship does not + assertThat(version.getPublishedWithId()).isNull(); + assertThat(version.getPublishedBy()).isNotNull(); + assertThat(version.getPublishedWithTt()).isEqualTo(PersonalAccessTokenType.LLT); + }); + + cleanUpVersion(versionId); + cleanUpUser("provenance-user"); + } + + // The link from a published artifact back to the workflow run is most of what trusted publishing buys + // over a token pasted into CI, and the token that carried it is deleted the moment it is used. + @Test + void aVersionKeepsItsTrustedPublishingProvenanceAfterTheTokenIsGone() { + var claims = Map.of( + "repository_id", + "74", + "repository_owner_id", + "65", + "workflow_ref", + "octo-org/octo-repo/.github/workflows/publish.yml@refs/tags/v1.2.0"); + var tokenId = persistToken("provenance-tpt", PersonalAccessTokenType.TPT, 1, null); + new TransactionTemplate(txManager) + .executeWithoutResult(status -> em.find(PersonalAccessToken.class, tokenId).setClaims(claims)); + var versionId = persistVersionPublishedWith(tokenId); + + // using a one-time token deletes it, so the copy is the only thing left + new TransactionTemplate(txManager) + .executeWithoutResult(status -> em.remove(em.find(PersonalAccessToken.class, tokenId))); + + new TransactionTemplate(txManager).executeWithoutResult(status -> { + var version = em.find(ExtensionVersion.class, versionId); + assertThat(version.getPublishedProvenance()).isEqualTo(claims); + assertThat(version.getPublishedWithId()).isNull(); + }); + + cleanUpVersion(versionId); + cleanUpUser("provenance-tpt-user"); + } + + private long persistVersionPublishedWith(long tokenId) { + return new TransactionTemplate(txManager).execute(status -> { + var token = em.find(PersonalAccessToken.class, tokenId); + + var namespace = new Namespace(); + namespace.setName("provenance-ns-" + tokenId); + em.persist(namespace); + + var extension = new Extension(); + extension.setName("provenance-ext-" + tokenId); + extension.setNamespace(namespace); + extension.setActive(true); + em.persist(extension); + + var version = new ExtensionVersion(); + version.setExtension(extension); + version.setVersion("1.0.0"); + version.setTargetPlatform(TargetPlatform.NAME_UNIVERSAL); + version.setTimestamp(TimeUtil.getCurrentUTC()); + version.setPublishedBy(token.getUser()); + version.setPublishedWithTt(token.getType()); + version.setPublishedWithId(token.getId()); + version.setPublishedProvenance(token.getClaims()); + em.persist(version); + em.flush(); + return version.getId(); + }); + } + + private void cleanUpVersion(long versionId) { + new TransactionTemplate(txManager).executeWithoutResult(status -> { + var version = em.find(ExtensionVersion.class, versionId); + var extension = version.getExtension(); + var namespaceId = extension.getNamespace().getId(); + em.remove(version); + em.flush(); + em.createQuery("delete from Extension e where e.id = :id").setParameter("id", extension.getId()) + .executeUpdate(); + em.createQuery("delete from Namespace n where n.id = :id").setParameter("id", namespaceId) + .executeUpdate(); + }); + } + + private long persistToken(String name, PersonalAccessTokenType type, int version, LocalDateTime expires) { + return new TransactionTemplate(txManager).execute(status -> { + var user = new UserData(); + user.setLoginName(name + "-user"); + user.setProvider("github"); + em.persist(user); + + var token = new PersonalAccessToken(); + token.setUser(user); + token.setValue(name + "-token-value"); + token.setActive(true); + token.setCreatedTimestamp(TimeUtil.getCurrentUTC()); + token.setExpiresTimestamp(expires); + token.setVersion(version); + token.setType(type); + token.setDescription(name); + em.persist(token); + em.flush(); + return token.getId(); + }); + } + + private long persistLegacyToken() { + return new TransactionTemplate(txManager).execute(status -> { + var user = new UserData(); + user.setLoginName("concurrent-write-user"); + user.setProvider("github"); + em.persist(user); + + var token = new PersonalAccessToken(); + token.setUser(user); + token.setValue("concurrent-write-token-value"); + token.setActive(true); + token.setCreatedTimestamp(TimeUtil.getCurrentUTC()); + token.setVersion(0); + token.setType(PersonalAccessTokenType.LLT); + token.setDescription("legacy token"); + em.persist(token); + em.flush(); + return token.getId(); + }); + } + + private void revokeInAnotherTransaction(long tokenId) { + var requiresNew = new TransactionTemplate(txManager); + requiresNew.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW); + requiresNew.executeWithoutResult( + status -> em.createNativeQuery("UPDATE personal_access_token SET active = false WHERE id = :id") + .setParameter("id", tokenId) + .executeUpdate()); + } + + private void cleanUpUser(String loginName) { + new TransactionTemplate(txManager).executeWithoutResult( + status -> em.createQuery("delete from UserData u where u.loginName = :name") + .setParameter("name", loginName) + .executeUpdate()); + } + + /** The container is shared with every other test, so leave nothing of this one behind. */ + private void cleanUp(long tokenId) { + new TransactionTemplate(txManager).executeWithoutResult(status -> { + var token = em.find(PersonalAccessToken.class, tokenId); + var userId = token.getUser().getId(); + em.remove(token); + em.flush(); + em.createQuery("delete from UserData u where u.id = :id").setParameter("id", userId).executeUpdate(); + }); + } +} diff --git a/server/src/test/java/org/eclipse/openvsx/accesstoken/AccessTokenServiceTest.java b/server/src/test/java/org/eclipse/openvsx/accesstoken/AccessTokenServiceTest.java index bbc2912ae..2ba63edc4 100644 --- a/server/src/test/java/org/eclipse/openvsx/accesstoken/AccessTokenServiceTest.java +++ b/server/src/test/java/org/eclipse/openvsx/accesstoken/AccessTokenServiceTest.java @@ -12,23 +12,37 @@ *****************************************************************************/ package org.eclipse.openvsx.accesstoken; +import java.time.Duration; +import java.time.LocalDateTime; +import java.time.ZoneId; +import java.util.Map; + import jakarta.persistence.EntityManager; +import org.jooq.DSLContext; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.data.util.Streamable; +import org.eclipse.openvsx.entities.Extension; +import org.eclipse.openvsx.entities.Namespace; import org.eclipse.openvsx.entities.PersonalAccessToken; import org.eclipse.openvsx.entities.PersonalAccessTokenType; +import org.eclipse.openvsx.entities.TrustedPublisher; import org.eclipse.openvsx.entities.UserData; import org.eclipse.openvsx.mail.MailService; import org.eclipse.openvsx.repositories.RepositoryService; -import org.eclipse.openvsx.util.UUIDService; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) @@ -37,9 +51,6 @@ class AccessTokenServiceTest { @Mock AccessTokenConfig config; - @Mock - UUIDService uuidService; - @Mock EntityManager entityManager; @@ -49,13 +60,17 @@ class AccessTokenServiceTest { @Mock MailService mail; + @Mock + DSLContext dsl; + @InjectMocks AccessTokenService accessTokenService; @BeforeEach void setUp() { - when(config.getTokenHashAlgorithm()).thenReturn("SHA-256"); - when(config.getTokenHashSalt()).thenReturn("salt"); + // lenient: generateTokenValue does not hash anything, so it needs neither of these + lenient().when(config.getTokenHashAlgorithm()).thenReturn("SHA-256"); + lenient().when(config.getTokenHashSalt()).thenReturn("salt"); } private PersonalAccessToken activeUnrestrictedToken() { @@ -95,4 +110,102 @@ void rejectsATokenWithNoUser() { assertThat(tau).isNull(); } + + // The upgrade job is enqueued from every pod's own ApplicationStartedEvent, so the advisory lock is + // what keeps one rolling update from having each of them scan and rewrite the same rows. + @Test + void skipsTheTokenUpgradeWhenAnotherInstanceHoldsTheLock() { + var service = Mockito.spy(accessTokenService); + Mockito.doReturn(false).when(service).tryAcquireUpgradeLock(); + + assertThat(service.upgradeTokens()).isZero(); + + verifyNoInteractions(repositories); + } + + @Test + void upgradesTokensWhenItWinsTheLock() { + var service = Mockito.spy(accessTokenService); + Mockito.doReturn(true).when(service).tryAcquireUpgradeLock(); + var legacy = new PersonalAccessToken(); + legacy.setVersion(0); + legacy.setValue("raw"); + when(repositories.findAllPersonalAccessTokensByVersion(0)).thenReturn(Streamable.of(legacy)); + + assertThat(service.upgradeTokens()).isEqualTo(1); + + assertThat(legacy.getVersion()).isEqualTo(1); + assertThat(legacy.getValue()).isNotEqualTo("raw"); + } + + // The old implementation regenerated until repositories.hasPersonalAccessToken(value) came back + // false, comparing a raw value against a column that stores salted hashes - it could never match for + // a current token, so it only cost a query per token created. Uniqueness is UNIQUE (value)'s job, and + // only it can work across pods anyway. + @Test + void generatesATokenValueWithoutAskingTheDatabase() { + when(config.getPrefix()).thenReturn("ovsx"); + + var value = accessTokenService.generateTokenValue(PersonalAccessTokenType.LLT); + + // prefix, then the marker saying what kind of token this is, then 256 bits base64url encoded + assertThat(value).startsWith("ovsxat_"); + assertThat(value.substring("ovsxat_".length())).hasSize(43).doesNotContain("=", "+", "/"); + verifyNoInteractions(repositories); + } + + @Test + void marksEachKindOfTokenDistinctly() { + when(config.getPrefix()).thenReturn("ovsx"); + + assertThat(accessTokenService.generateTokenValue(PersonalAccessTokenType.TPT)).startsWith("ovsxtp_"); + assertThat(accessTokenService.generateTokenValue(PersonalAccessTokenType.LLT)).startsWith("ovsxat_"); + } + + @Test + void generatesADifferentValueEveryTime() { + when(config.getPrefix()).thenReturn(""); + + var values = java.util.stream.Stream.generate( + () -> accessTokenService.generateTokenValue(PersonalAccessTokenType.LLT)).limit(100).toList(); + + assertThat(values).doesNotHaveDuplicates(); + } + + // How long a publishing token lives is the trusted publishing configuration's to decide, so this + // service applies whatever it is handed instead of reading a setting of its own. + @Test + void appliesTheExpirationItIsGivenToATrustedPublishingToken() { + var user = new UserData(); + var namespace = new Namespace(); + namespace.setName("foo"); + var extension = new Extension(); + extension.setName("bar"); + extension.setNamespace(namespace); + var trustedPublisher = new TrustedPublisher(); + trustedPublisher.setExtension(extension); + trustedPublisher.setCreatedBy(user); + trustedPublisher.setRegistration(Map.of()); + when(config.getPrefix()).thenReturn("ovsx"); + + var before = LocalDateTime.now(ZoneId.of("UTC")); + var json = accessTokenService + .createTrustedPublishingAccessToken( + trustedPublisher, + "Trusted publishing (github)", + Duration.ofMinutes(7), + Map.of("repository_id", "74")); + var after = LocalDateTime.now(ZoneId.of("UTC")); + + var persisted = ArgumentCaptor.forClass(PersonalAccessToken.class); + verify(entityManager).persist(persisted.capture()); + assertThat(persisted.getValue().getType()).isEqualTo(PersonalAccessTokenType.TPT); + assertThat(persisted.getValue().getExpiresTimestamp()) + .isBetween(before.plusMinutes(7), after.plusMinutes(7)); + // the token is scoped to the registration's extension, whatever its lifetime + assertThat(persisted.getValue().getScopeExtension()).isSameAs(extension); + // carried to the publish that uses this token, which copies them onto the version + assertThat(persisted.getValue().getClaims()).containsEntry("repository_id", "74"); + assertThat(json.getValue()).isNotNull(); + } } diff --git a/server/src/test/java/org/eclipse/openvsx/admin/AdminAPITest.java b/server/src/test/java/org/eclipse/openvsx/admin/AdminAPITest.java index 51ffbb538..19aad3c1f 100644 --- a/server/src/test/java/org/eclipse/openvsx/admin/AdminAPITest.java +++ b/server/src/test/java/org/eclipse/openvsx/admin/AdminAPITest.java @@ -22,6 +22,8 @@ import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import jakarta.persistence.EntityManager; import org.jobrunr.scheduling.JobRequestScheduler; +import org.jooq.DSLContext; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mockito; import org.mockito.invocation.InvocationOnMock; @@ -127,6 +129,7 @@ @WebMvcTest(AdminAPI.class) @MockitoBean( types = { + DSLContext.class, ClientRegistrationRepository.class, UpstreamRegistryService.class, GoogleCloudStorageService.class, @@ -151,6 +154,14 @@ ) class AdminAPITest { + @BeforeEach + void noTrustedPublishersByDefault() { + // the real repository hands back an empty Streamable rather than null; only the trusted + // publishing tests care what it actually holds + Mockito.when(repositories.findTrustedPublishersByNamespaceAndCreatedBy(any(), any())) + .thenReturn(Streamable.empty()); + } + @MockitoSpyBean UserService users; @@ -2079,6 +2090,9 @@ void testRevokeBulkPublishers() throws Exception { membership2.setUser(user2); when(repositories.findMemberships(user2)) .thenReturn(Streamable.of(membership2)); + // revoking goes through UserService.removeNamespaceMember, which looks the membership up again + when(repositories.findMembership(user, namespace)).thenReturn(membership); + when(repositories.findMembership(user2, namespace)).thenReturn(membership2); var baseRequest = """ { @@ -2547,12 +2561,12 @@ AccessTokenConfig tokenConfig() { @Bean AccessTokenService tokenService( AccessTokenConfig config, - UUIDService uuidService, EntityManager entityManager, RepositoryService repositories, - MailService mailService + MailService mailService, + DSLContext dsl ) { - return new AccessTokenService(config, uuidService, entityManager, repositories, mailService); + return new AccessTokenService(config, entityManager, repositories, mailService, dsl); } @Bean diff --git a/server/src/test/java/org/eclipse/openvsx/admin/AdminServiceTest.java b/server/src/test/java/org/eclipse/openvsx/admin/AdminServiceTest.java index cb90a06f5..e94cf9e12 100644 --- a/server/src/test/java/org/eclipse/openvsx/admin/AdminServiceTest.java +++ b/server/src/test/java/org/eclipse/openvsx/admin/AdminServiceTest.java @@ -20,12 +20,14 @@ import org.springframework.data.util.Streamable; import org.eclipse.openvsx.ExtensionService; +import org.eclipse.openvsx.UserService; import org.eclipse.openvsx.eclipse.EclipseService; import org.eclipse.openvsx.entities.Extension; import org.eclipse.openvsx.entities.ExtensionVersion; import org.eclipse.openvsx.entities.ExtensionVersionChange; import org.eclipse.openvsx.entities.ExtensionVersionState; import org.eclipse.openvsx.entities.Namespace; +import org.eclipse.openvsx.entities.NamespaceMembership; import org.eclipse.openvsx.entities.UserData; import org.eclipse.openvsx.repositories.RepositoryService; import org.eclipse.openvsx.util.LogService; @@ -64,6 +66,9 @@ class AdminServiceTest { @Mock LogService logs; + @Mock + UserService users; + @Mock EntityManager entityManager; @@ -97,6 +102,30 @@ private void mockNoReferences(Extension extension) { when(repositories.findDependenciesReference(extension)).thenReturn(Streamable.empty()); } + @Test + void revokingPublisherContributionsRemovesEachMembershipThroughUserService() { + var user = new UserData(); + user.setLoginName("amy"); + var namespace = new Namespace(); + namespace.setName(NAMESPACE); + var membership = new NamespaceMembership(); + membership.setUser(user); + membership.setNamespace(namespace); + membership.setRole(NamespaceMembership.ROLE_OWNER); + + when(repositories.findUserByLoginName("github", "amy")).thenReturn(user); + when(repositories.findPersonalAccessTokens(user)).thenReturn(Streamable.empty()); + when(repositories.findVersionsByUser(user, true)).thenReturn(Streamable.empty()); + when(repositories.findMemberships(user)).thenReturn(Streamable.of(membership)); + + var result = adminService.revokePublisherContributions("github", "amy", admin); + + // handing over the row we already hold is what keeps one unremovable membership from aborting the + // whole revoke, and going through UserService is what deletes the trusted publishers with it + verify(users).removeNamespaceMembership(membership); + assertThat(result.getSuccess()).contains("removed 1 namespace memberships"); + } + @Test void revokingPublisherContributionsRecordsWhenTheVersionsStoppedBeingVisible() { var user = new UserData(); diff --git a/server/src/test/java/org/eclipse/openvsx/publish/PublishExtensionVersionConcurrencyTest.java b/server/src/test/java/org/eclipse/openvsx/publish/PublishExtensionVersionConcurrencyTest.java index bd0ac7a89..4639beaff 100644 --- a/server/src/test/java/org/eclipse/openvsx/publish/PublishExtensionVersionConcurrencyTest.java +++ b/server/src/test/java/org/eclipse/openvsx/publish/PublishExtensionVersionConcurrencyTest.java @@ -286,7 +286,11 @@ private ExtensionVersion publish(String targetPlatform) { return publishHandler .createExtensionVersion( processor, - new AccessTokenAuthentication(publishToken().getUser(), publishToken().getType()), + new AccessTokenAuthentication( + publishToken().getUser(), + publishToken().getType(), + publishToken().getId(), + null), LocalDateTime.now(), false); } diff --git a/server/src/test/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandlerTest.java b/server/src/test/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandlerTest.java index 5afb3b26e..c0da662c8 100644 --- a/server/src/test/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandlerTest.java +++ b/server/src/test/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandlerTest.java @@ -167,7 +167,7 @@ void shouldRecordTheTokenTypeUsedToPublish() throws IOException { var namespace = buildNamespace("publisher"); var user = new UserData(); - var ata = new AccessTokenAuthentication(user, PersonalAccessTokenType.TPT); + var ata = new AccessTokenAuthentication(user, PersonalAccessTokenType.TPT, 1L, null); when(repositories.findNamespace("publisher")).thenReturn(namespace); when(users.hasPublishPermission(user, namespace)).thenReturn(true); diff --git a/server/src/test/java/org/eclipse/openvsx/repositories/RepositoryServiceSmokeTest.java b/server/src/test/java/org/eclipse/openvsx/repositories/RepositoryServiceSmokeTest.java index 4f224fe05..cfdd5642a 100644 --- a/server/src/test/java/org/eclipse/openvsx/repositories/RepositoryServiceSmokeTest.java +++ b/server/src/test/java/org/eclipse/openvsx/repositories/RepositoryServiceSmokeTest.java @@ -240,7 +240,6 @@ void testExecuteQueries() { () -> repositories.findMembership(userData, namespace), () -> repositories.findMemberships(namespace), () -> repositories.findMemberships(namespace, "role"), - () -> repositories.deleteMemberships(userData), () -> repositories.findNamespace("name"), () -> repositories.lockNamespace(namespace), () -> repositories.findConflictingNamespaces("displayName", namespace), @@ -351,7 +350,6 @@ void testExecuteQueries() { () -> repositories .findFirstUnresolvedDependency(List.of(new ExtensionId("namespaceName", "extensionName"))), () -> repositories.findAllPersonalAccessTokens(), - () -> repositories.hasPersonalAccessToken("tokenValue"), () -> repositories .findSignatureKeyPairPublicId("namespaceName", "extensionName", "targetPlatform", "version"), () -> repositories.findFirstMembership("namespaceName"), @@ -373,6 +371,8 @@ void testExecuteQueries() { () -> repositories.isDeleteAllActiveVersions("namespaceName", "extensionName"), () -> repositories.deactivatePersonalAccessTokens(userData), () -> repositories.expirePersonalAccessTokens(NOW), + () -> repositories + .deleteExpiredPersonalAccessTokens(NOW, List.of(PersonalAccessTokenType.TPT)), () -> repositories.findExpiringPersonalAccessTokensWithoutNotification(NOW, page), () -> repositories.updateExpiresTimeForLegacyPersonalAccessTokens(NOW, PersonalAccessTokenType.LLT), () -> repositories.findSimilarExtensionsByLevenshtein( @@ -490,6 +490,7 @@ void testExecuteQueries() { () -> repositories.findUnprocessedDaysForDailyUsage(customer), () -> repositories.saveDailyUsageStats(dailyUsageStats), () -> repositories.findTrustedPublishersByExtension(extension), + () -> repositories.findTrustedPublishersByNamespaceAndCreatedBy(namespace, userData), () -> repositories.findTrustedPublisher(1L), () -> repositories.deleteTrustedPublisher(trustedPublisher), () -> repositories.deleteTier(tier), diff --git a/server/src/test/java/org/eclipse/openvsx/trustedpublishing/TrustedPublisherCascadeTest.java b/server/src/test/java/org/eclipse/openvsx/trustedpublishing/TrustedPublisherCascadeTest.java new file mode 100644 index 000000000..053239c14 --- /dev/null +++ b/server/src/test/java/org/eclipse/openvsx/trustedpublishing/TrustedPublisherCascadeTest.java @@ -0,0 +1,190 @@ +/****************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0. + *****************************************************************************/ +package org.eclipse.openvsx.trustedpublishing; + +import java.util.Map; + +import jakarta.persistence.EntityManager; +import org.jobrunr.scheduling.JobRequestScheduler; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.bean.override.mockito.MockitoBean; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.support.TransactionTemplate; + +import org.eclipse.openvsx.AbstractPostgresContainerTest; +import org.eclipse.openvsx.ExtensionService; +import org.eclipse.openvsx.entities.Extension; +import org.eclipse.openvsx.entities.Namespace; +import org.eclipse.openvsx.entities.PersonalAccessToken; +import org.eclipse.openvsx.entities.PersonalAccessTokenType; +import org.eclipse.openvsx.entities.TrustedPublisher; +import org.eclipse.openvsx.entities.UserData; +import org.eclipse.openvsx.repositories.RepositoryService; +import org.eclipse.openvsx.search.SearchUtilService; +import org.eclipse.openvsx.util.TimeUtil; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; + +/** + * A trusted publisher points at an extension, and the publishing token issued under it points at both, so + * deleting either end has to take what depends on it along. Only the database can be asserted on: none of + * these references has an inverse mapping on {@link Extension} or {@link TrustedPublisher}, so JPA cascades + * nothing and the foreign key constraints alone decide what happens. + */ +@SpringBootTest +class TrustedPublisherCascadeTest extends AbstractPostgresContainerTest { + + @Autowired + ExtensionService extensions; + + @Autowired + RepositoryService repositories; + + @Autowired + EntityManager em; + + @Autowired + PlatformTransactionManager txManager; + + @MockitoBean + SearchUtilService search; + + @MockitoBean + JobRequestScheduler scheduler; + + @Test + void purgingAnExtensionTakesItsTrustedPublisherAndTokensWithIt() { + var ids = persistRegistrationWithToken("purge"); + + // without the constraints deleting the registration and the token, this fails outright on a foreign + // key and the extension can never be purged at all + assertThatCode(() -> transaction(status -> { + var user = em.find(UserData.class, ids.userId()); + var extension = em.find(Extension.class, ids.extensionId()); + extensions.purgeExtension(user, extension, false); + })).doesNotThrowAnyException(); + + transaction(status -> { + assertThat(em.find(Extension.class, ids.extensionId())).isNull(); + assertThat(em.find(TrustedPublisher.class, ids.trustedPublisherId())).isNull(); + // retiring the token rather than detaching it: one left behind with neither scope set would + // read as unrestricted + assertThat(em.find(PersonalAccessToken.class, ids.tokenId())).isNull(); + }); + + cleanUp(ids, "purge"); + } + + @Test + void deletingATrustedPublisherTakesTheTokensIssuedUnderItWithIt() { + var ids = persistRegistrationWithToken("revoke"); + + transaction( + status -> repositories + .deleteTrustedPublisher(em.find(TrustedPublisher.class, ids.trustedPublisherId()))); + + transaction(status -> { + assertThat(em.find(TrustedPublisher.class, ids.trustedPublisherId())).isNull(); + // a token issued under a registration may only publish the extension it was made for, so it + // has nothing left to authorize once the registration is gone + assertThat(em.find(PersonalAccessToken.class, ids.tokenId())).isNull(); + // the extension itself is untouched by this + assertThat(em.find(Extension.class, ids.extensionId())).isNotNull(); + }); + + cleanUp(ids, "revoke"); + } + + private record Ids(long userId, long namespaceId, long extensionId, long trustedPublisherId, long tokenId) {} + + /** + * A registration that has actually been used: the exchange leaves a token scoped to the same extension + * behind, and expiry only deactivates that row, so it outlives the exchange it was issued for. + */ + private Ids persistRegistrationWithToken(String prefix) { + return new TransactionTemplate(txManager).execute(status -> { + var user = new UserData(); + user.setLoginName(prefix + "-tp-owner"); + user.setProvider("github"); + em.persist(user); + + var namespace = new Namespace(); + namespace.setName(prefix + "-tp-testns"); + em.persist(namespace); + + var extension = new Extension(); + extension.setName(prefix + "-tp-testext"); + extension.setNamespace(namespace); + extension.setActive(true); + em.persist(extension); + + var trustedPublisher = new TrustedPublisher(); + trustedPublisher.setExtension(extension); + trustedPublisher.setProvider("github"); + trustedPublisher.setRegistration(Map.of("owner", "octo-org", "repo", "octo-repo")); + trustedPublisher.setClaims(Map.of("repository_id", "74")); + trustedPublisher.setCreatedBy(user); + trustedPublisher.setCreatedTimestamp(TimeUtil.getCurrentUTC()); + em.persist(trustedPublisher); + + var token = new PersonalAccessToken(); + token.setUser(user); + token.setValue(prefix + "-tp-token-value"); + token.setActive(false); + token.setCreatedTimestamp(TimeUtil.getCurrentUTC()); + token.setVersion(1); + token.setType(PersonalAccessTokenType.TPT); + token.setDescription("Trusted publishing (github)"); + token.setTrustedPublisher(trustedPublisher); + token.setScopeExtension(extension); + em.persist(token); + em.flush(); + + return new Ids( + user.getId(), + namespace.getId(), + extension.getId(), + trustedPublisher.getId(), + token.getId()); + }); + } + + private void transaction(java.util.function.Consumer work) { + new TransactionTemplate(txManager).executeWithoutResult(work::accept); + } + + /** The container is shared with every other test, so leave nothing of this one behind. */ + private void cleanUp(Ids ids, String prefix) { + transaction(status -> { + em.createQuery("delete from PersonalAccessToken t where t.user.id = :id") + .setParameter("id", ids.userId()) + .executeUpdate(); + em.createQuery("delete from TrustedPublisher p where p.createdBy.id = :id") + .setParameter("id", ids.userId()) + .executeUpdate(); + em.createQuery("delete from Extension e where e.namespace.id = :id") + .setParameter("id", ids.namespaceId()) + .executeUpdate(); + em.createQuery("delete from PersistedLog l where l.user.id = :id") + .setParameter("id", ids.userId()) + .executeUpdate(); + em.createQuery("delete from Namespace n where n.id = :id") + .setParameter("id", ids.namespaceId()) + .executeUpdate(); + em.createQuery("delete from UserData u where u.id = :id") + .setParameter("id", ids.userId()) + .executeUpdate(); + }); + } +} diff --git a/server/src/test/java/org/eclipse/openvsx/trustedpublishing/TrustedPublishingAPITest.java b/server/src/test/java/org/eclipse/openvsx/trustedpublishing/TrustedPublishingAPITest.java new file mode 100644 index 000000000..0bae8874c --- /dev/null +++ b/server/src/test/java/org/eclipse/openvsx/trustedpublishing/TrustedPublishingAPITest.java @@ -0,0 +1,562 @@ +/****************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + *****************************************************************************/ +package org.eclipse.openvsx.trustedpublishing; + +import java.time.LocalDateTime; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import io.micrometer.core.instrument.MeterRegistry; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.security.oauth2.client.autoconfigure.servlet.OAuth2ClientWebSecurityAutoConfiguration; +import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc; +import org.springframework.boot.webmvc.test.autoconfigure.WebMvcTest; +import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; +import org.springframework.test.context.bean.override.mockito.MockitoBean; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; +import tools.jackson.databind.json.JsonMapper; + +import org.eclipse.openvsx.UserService; +import org.eclipse.openvsx.eclipse.EclipseService; +import org.eclipse.openvsx.entities.Extension; +import org.eclipse.openvsx.entities.Namespace; +import org.eclipse.openvsx.entities.TrustedPublisher; +import org.eclipse.openvsx.entities.UserData; +import org.eclipse.openvsx.json.AccessTokenJson; +import org.eclipse.openvsx.json.ResultJson; +import org.eclipse.openvsx.json.TrustedPublisherInputJson; +import org.eclipse.openvsx.trustedpublishing.TrustedPublishingService.TrustedPublishers; +import org.eclipse.openvsx.util.ErrorResultException; +import org.eclipse.openvsx.util.NotFoundException; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +@WebMvcTest( + value = TrustedPublishingAPI.class, + excludeAutoConfiguration = { OAuth2ClientWebSecurityAutoConfiguration.class } +) +@AutoConfigureMockMvc(addFilters = false) +class TrustedPublishingAPITest { + + private static final String NAMESPACE = "foo"; + + private static final String EXTENSION = "bar"; + + private static final String PROVIDER = "github"; + + private static final Map REGISTRATION = Map + .of("owner", "foo-org", "repository", "bar-repo", "workflow", "publish.yml"); + + @Autowired + MockMvc mockMvc; + + @MockitoBean + MeterRegistry meterRegistry; + + @MockitoBean + UserService users; + + @MockitoBean + EclipseService eclipseService; + + @MockitoBean + TrustedPublishingService trustedPublishing; + + // --------------------------------------------------------------------------------------- + // POST /user/namespace/{namespace}/trusted-publishing/create + // --------------------------------------------------------------------------------------- + + @Test + void createTrustedPublisher_returns403_whenNotLoggedIn() throws Exception { + mockMvc.perform(createRequest(NAMESPACE, registrationBody(NAMESPACE, EXTENSION, PROVIDER, REGISTRATION))) + .andExpect(status().isForbidden()); + + verifyNoInteractions(trustedPublishing); + } + + @Test + void createTrustedPublisher_returns400_whenMandatoryFieldsAreMissing() throws Exception { + mockLoggedInUser(); + + var missingProvider = registrationBody(NAMESPACE, EXTENSION, null, REGISTRATION); + var missingExtension = registrationBody(NAMESPACE, null, PROVIDER, REGISTRATION); + var missingNamespace = registrationBody(null, EXTENSION, PROVIDER, REGISTRATION); + var missingRegistration = registrationBody(NAMESPACE, EXTENSION, PROVIDER, Map.of()); + + for (var body : List.of(missingProvider, missingExtension, missingNamespace, missingRegistration)) { + mockMvc.perform(createRequest(NAMESPACE, body)) + .andExpect(status().isBadRequest()) + .andExpect( + jsonPath("$.error") + .value( + "The fields provider, namespace, extension and registration are mandatory.")); + } + + verify(trustedPublishing, never()) + .registerTrustedPublisher(any(), anyString(), anyString(), anyString(), any()); + } + + @Test + void createTrustedPublisher_returns400_whenNamespaceDoesNotMatchPath() throws Exception { + mockLoggedInUser(); + + mockMvc.perform(createRequest("other", registrationBody(NAMESPACE, EXTENSION, PROVIDER, REGISTRATION))) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error").value("The namespace in the path and in the request body must match.")); + + verify(trustedPublishing, never()) + .registerTrustedPublisher(any(), anyString(), anyString(), anyString(), any()); + } + + @Test + void createTrustedPublisher_returns201_andRegisteredPublisher() throws Exception { + var user = mockLoggedInUser(); + var publisher = trustedPublisher(7L); + when(trustedPublishing.registerTrustedPublisher(user, NAMESPACE, EXTENSION, PROVIDER, REGISTRATION)) + .thenReturn(publisher); + + mockMvc.perform(createRequest(NAMESPACE, registrationBody(NAMESPACE, EXTENSION, PROVIDER, REGISTRATION))) + .andExpect(status().isCreated()) + .andExpect(jsonPath("$.id").value(7)) + .andExpect(jsonPath("$.namespace").value(NAMESPACE)) + .andExpect(jsonPath("$.extension").value(EXTENSION)) + .andExpect(jsonPath("$.provider").value(PROVIDER)) + .andExpect(jsonPath("$.registration.owner").value("foo-org")) + .andExpect(jsonPath("$.registration.repository").value("bar-repo")) + .andExpect(jsonPath("$.registration.workflow").value("publish.yml")) + .andExpect(jsonPath("$.createdTimestamp").value("2026-01-02T03:04:05Z")) + .andExpect(jsonPath("$.error").doesNotExist()); + + verify(eclipseService).checkPublisherAgreement(user); + } + + @Test + void createTrustedPublisher_returns403_whenPublisherAgreementIsMissing() throws Exception { + var user = mockLoggedInUser(); + doThrow( + new ErrorResultException( + "You must sign a Publisher Agreement with the Eclipse Foundation before publishing any extension.", + HttpStatus.FORBIDDEN)) + .when(eclipseService) + .checkPublisherAgreement(user); + + mockMvc.perform(createRequest(NAMESPACE, registrationBody(NAMESPACE, EXTENSION, PROVIDER, REGISTRATION))) + .andExpect(status().isForbidden()) + .andExpect( + jsonPath("$.error") + .value( + "You must sign a Publisher Agreement with the Eclipse Foundation before publishing any extension.")); + + verify(trustedPublishing, never()) + .registerTrustedPublisher(any(), anyString(), anyString(), anyString(), any()); + } + + @Test + void createTrustedPublisher_returns404_whenNamespaceIsUnknown() throws Exception { + var user = mockLoggedInUser(); + when(trustedPublishing.registerTrustedPublisher(user, NAMESPACE, EXTENSION, PROVIDER, REGISTRATION)) + .thenThrow(new NotFoundException()); + + mockMvc.perform(createRequest(NAMESPACE, registrationBody(NAMESPACE, EXTENSION, PROVIDER, REGISTRATION))) + .andExpect(status().isNotFound()) + .andExpect(jsonPath("$.error").value("Namespace not found: " + NAMESPACE)); + } + + @Test + void createTrustedPublisher_returns403_whenUserDoesNotOwnNamespace() throws Exception { + var user = mockLoggedInUser(); + when(trustedPublishing.registerTrustedPublisher(user, NAMESPACE, EXTENSION, PROVIDER, REGISTRATION)) + .thenThrow(new ErrorResultException("You must be an owner of this namespace.", HttpStatus.FORBIDDEN)); + + mockMvc.perform(createRequest(NAMESPACE, registrationBody(NAMESPACE, EXTENSION, PROVIDER, REGISTRATION))) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.error").value("You must be an owner of this namespace.")); + } + + @Test + void createTrustedPublisher_returns400_whenRegistrationIsRejected() throws Exception { + var user = mockLoggedInUser(); + when(trustedPublishing.registerTrustedPublisher(user, NAMESPACE, EXTENSION, PROVIDER, REGISTRATION)) + .thenThrow(new ErrorResultException("An equivalent trusted publisher is already registered.")); + + mockMvc.perform(createRequest(NAMESPACE, registrationBody(NAMESPACE, EXTENSION, PROVIDER, REGISTRATION))) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error").value("An equivalent trusted publisher is already registered.")); + } + + // --------------------------------------------------------------------------------------- + // GET /user/namespace/{namespace}/trusted-publishing + // --------------------------------------------------------------------------------------- + + @Test + void getTrustedPublishers_returns403_whenNotLoggedIn() throws Exception { + mockMvc.perform(get("/user/namespace/{namespace}/trusted-publishing", NAMESPACE)) + .andExpect(status().isForbidden()); + + verifyNoInteractions(trustedPublishing); + } + + @Test + void getTrustedPublishers_returnsPublishersAndRegistrableExtensions() throws Exception { + var user = mockLoggedInUser(); + when(trustedPublishing.getTrustedPublishers(user, NAMESPACE)) + .thenReturn(new TrustedPublishers(List.of(trustedPublisher(7L)), List.of("baz", "qux"))); + + mockMvc.perform(get("/user/namespace/{namespace}/trusted-publishing", NAMESPACE)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.trustedPublishers.length()").value(1)) + .andExpect(jsonPath("$.trustedPublishers[0].id").value(7)) + .andExpect(jsonPath("$.trustedPublishers[0].namespace").value(NAMESPACE)) + .andExpect(jsonPath("$.trustedPublishers[0].extension").value(EXTENSION)) + .andExpect(jsonPath("$.trustedPublishers[0].provider").value(PROVIDER)) + .andExpect(jsonPath("$.registrableExtensions.length()").value(2)) + .andExpect(jsonPath("$.registrableExtensions[0]").value("baz")) + .andExpect(jsonPath("$.registrableExtensions[1]").value("qux")) + .andExpect(jsonPath("$.error").doesNotExist()); + } + + @Test + void getTrustedPublishers_returnsEmptyLists_whenNothingIsRegistered() throws Exception { + var user = mockLoggedInUser(); + when(trustedPublishing.getTrustedPublishers(user, NAMESPACE)) + .thenReturn(new TrustedPublishers(List.of(), List.of())); + + mockMvc.perform(get("/user/namespace/{namespace}/trusted-publishing", NAMESPACE)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.trustedPublishers.length()").value(0)) + .andExpect(jsonPath("$.registrableExtensions.length()").value(0)); + } + + @Test + void getTrustedPublishers_returns404_whenNamespaceIsUnknown() throws Exception { + var user = mockLoggedInUser(); + when(trustedPublishing.getTrustedPublishers(user, NAMESPACE)).thenThrow(new NotFoundException()); + + mockMvc.perform(get("/user/namespace/{namespace}/trusted-publishing", NAMESPACE)) + .andExpect(status().isNotFound()) + .andExpect(jsonPath("$.error").value("Namespace not found: " + NAMESPACE)); + } + + @Test + void getTrustedPublishers_returns404_whenFeatureIsDisabled() throws Exception { + var user = mockLoggedInUser(); + when(trustedPublishing.getTrustedPublishers(user, NAMESPACE)) + .thenThrow(new ErrorResultException("Trusted publishing is not enabled.", HttpStatus.NOT_FOUND)); + + mockMvc.perform(get("/user/namespace/{namespace}/trusted-publishing", NAMESPACE)) + .andExpect(status().isNotFound()) + .andExpect(jsonPath("$.error").value("Trusted publishing is not enabled.")); + } + + @Test + void getTrustedPublishers_returns403_whenUserDoesNotOwnNamespace() throws Exception { + var user = mockLoggedInUser(); + when(trustedPublishing.getTrustedPublishers(user, NAMESPACE)) + .thenThrow(new ErrorResultException("You must be an owner of this namespace.", HttpStatus.FORBIDDEN)); + + mockMvc.perform(get("/user/namespace/{namespace}/trusted-publishing", NAMESPACE)) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.error").value("You must be an owner of this namespace.")); + } + + // --------------------------------------------------------------------------------------- + // POST /user/namespace/{namespace}/trusted-publishing/delete/{id} + // --------------------------------------------------------------------------------------- + + @Test + void deleteTrustedPublisher_returns403_whenNotLoggedIn() throws Exception { + mockMvc.perform(post("/user/namespace/{namespace}/trusted-publishing/delete/{id}", NAMESPACE, 7)) + .andExpect(status().isForbidden()); + + verifyNoInteractions(trustedPublishing); + } + + @Test + void deleteTrustedPublisher_returns200_andSuccessMessage() throws Exception { + var user = mockLoggedInUser(); + when(trustedPublishing.deleteTrustedPublisher(user, NAMESPACE, 7L)) + .thenReturn(ResultJson.success("Deleted trusted publisher for namespace " + NAMESPACE + ".")); + + mockMvc.perform(post("/user/namespace/{namespace}/trusted-publishing/delete/{id}", NAMESPACE, 7)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.success").value("Deleted trusted publisher for namespace " + NAMESPACE + ".")) + .andExpect(jsonPath("$.error").doesNotExist()); + + verify(trustedPublishing).deleteTrustedPublisher(user, NAMESPACE, 7L); + } + + @Test + void deleteTrustedPublisher_returns404_whenPublisherIsUnknown() throws Exception { + var user = mockLoggedInUser(); + when(trustedPublishing.deleteTrustedPublisher(user, NAMESPACE, 7L)).thenThrow(new NotFoundException()); + + mockMvc.perform(post("/user/namespace/{namespace}/trusted-publishing/delete/{id}", NAMESPACE, 7)) + .andExpect(status().isNotFound()) + .andExpect(jsonPath("$.error").value("Trusted publisher does not exist.")); + } + + @Test + void deleteTrustedPublisher_returns403_whenUserDoesNotOwnNamespace() throws Exception { + var user = mockLoggedInUser(); + when(trustedPublishing.deleteTrustedPublisher(user, NAMESPACE, 7L)) + .thenThrow(new ErrorResultException("You must be an owner of this namespace.", HttpStatus.FORBIDDEN)); + + mockMvc.perform(post("/user/namespace/{namespace}/trusted-publishing/delete/{id}", NAMESPACE, 7)) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.error").value("You must be an owner of this namespace.")); + } + + // --------------------------------------------------------------------------------------- + // POST /api/-/trusted-publishing/token + // --------------------------------------------------------------------------------------- + + @Test + void requestPublishToken_returns400_whenMandatoryFieldsAreMissing() throws Exception { + var missingNamespace = tokenRequestBody(null, EXTENSION, "the-token"); + var missingExtension = tokenRequestBody(NAMESPACE, null, "the-token"); + var missingToken = tokenRequestBody(NAMESPACE, EXTENSION, null); + + for (var body : List.of(missingNamespace, missingExtension, missingToken)) { + mockMvc.perform(tokenRequest(body)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error").value("The fields namespace, extension and token are mandatory.")); + } + + verify(trustedPublishing, never()).requestPublishToken(anyString(), anyString(), anyString()); + } + + @Test + void requestPublishToken_returns201_andAccessToken() throws Exception { + // the exchange authenticates through the presented OIDC token, so no logged-in user is needed + var accessToken = new AccessTokenJson(); + accessToken.setId(42L); + accessToken.setValue("the-access-token"); + accessToken.setDescription("Trusted publishing (github)"); + when(trustedPublishing.requestPublishToken(NAMESPACE, EXTENSION, "the-token")).thenReturn(accessToken); + + mockMvc.perform(tokenRequest(tokenRequestBody(NAMESPACE, EXTENSION, "the-token"))) + .andExpect(status().isCreated()) + .andExpect(jsonPath("$.id").value(42)) + .andExpect(jsonPath("$.value").value("the-access-token")) + .andExpect(jsonPath("$.description").value("Trusted publishing (github)")) + .andExpect(jsonPath("$.error").doesNotExist()); + + verifyNoInteractions(users); + } + + @Test + void requestPublishToken_returns403_whenNoTrustedPublisherMatches() throws Exception { + when(trustedPublishing.requestPublishToken(NAMESPACE, EXTENSION, "the-token")).thenThrow( + new ErrorResultException( + "No trusted publisher matches the presented token.", + HttpStatus.FORBIDDEN)); + + mockMvc.perform(tokenRequest(tokenRequestBody(NAMESPACE, EXTENSION, "the-token"))) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.error").value("No trusted publisher matches the presented token.")); + } + + @Test + void requestPublishToken_returns400_whenTokenIsNotAcceptable() throws Exception { + when(trustedPublishing.requestPublishToken(NAMESPACE, EXTENSION, "the-token")) + .thenThrow(new ErrorResultException("Unsupported token issuer.")); + + mockMvc.perform(tokenRequest(tokenRequestBody(NAMESPACE, EXTENSION, "the-token"))) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error").value("Unsupported token issuer.")); + } + + // --------------------------------------------------------------------------------------- + // GET /api/-/trusted-publishing/status + // --------------------------------------------------------------------------------------- + + @Test + void getTrustedPublishingStatus_returns403_whenNotLoggedIn() throws Exception { + mockMvc.perform(get("/api/-/trusted-publishing/status")).andExpect(status().isForbidden()); + + verifyNoInteractions(trustedPublishing); + } + + @Test + void getTrustedPublishingStatus_reportsDisabledFeature() throws Exception { + mockLoggedInUser(); + when(trustedPublishing.isEnabled()).thenReturn(false); + + mockMvc.perform(get("/api/-/trusted-publishing/status")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.enabled").value(false)) + .andExpect(jsonPath("$.allowed").value(false)) + .andExpect(jsonPath("$.trustedPublisherProviders").doesNotExist()); + + verify(trustedPublishing, never()).getTrustedPublisherProviders(); + } + + @Test + void getTrustedPublishingStatus_hidesProviders_whenPublisherAgreementIsMissing() throws Exception { + var user = mockLoggedInUser(); + when(trustedPublishing.isEnabled()).thenReturn(true); + when(eclipseService.hasPublisherAgreement(user)).thenReturn(false); + + mockMvc.perform(get("/api/-/trusted-publishing/status")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.enabled").value(true)) + .andExpect(jsonPath("$.allowed").value(false)) + .andExpect(jsonPath("$.trustedPublisherProviders").doesNotExist()); + + verify(trustedPublishing, never()).getTrustedPublisherProviders(); + } + + @Test + void getTrustedPublishingStatus_listsActiveProviders() throws Exception { + var user = mockLoggedInUser(); + when(trustedPublishing.isEnabled()).thenReturn(true); + when(eclipseService.hasPublisherAgreement(user)).thenReturn(true); + + // LinkedHashMap so the order of the listed providers is the one asserted below + var providers = new LinkedHashMap(); + providers.put( + PROVIDER, + provider( + PROVIDER, + "GitHub Actions", + "https://github.com", + List.of( + TrustedPublisherInputJson.create("owner", "The owner of the repository", false), + TrustedPublisherInputJson.create("environment", "The environment", true)))); + providers.put("gitlab", provider("gitlab", "GitLab CI/CD", "https://gitlab.com", List.of())); + when(trustedPublishing.getTrustedPublisherProviders()).thenReturn(providers); + + mockMvc.perform(get("/api/-/trusted-publishing/status")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.enabled").value(true)) + .andExpect(jsonPath("$.allowed").value(true)) + .andExpect(jsonPath("$.trustedPublisherProviders.length()").value(2)) + .andExpect(jsonPath("$.trustedPublisherProviders[0].id").value(PROVIDER)) + .andExpect(jsonPath("$.trustedPublisherProviders[0].name").value("GitHub Actions")) + .andExpect(jsonPath("$.trustedPublisherProviders[0].url").value("https://github.com")) + .andExpect(jsonPath("$.trustedPublisherProviders[0].registrationInputs.length()").value(2)) + .andExpect(jsonPath("$.trustedPublisherProviders[0].registrationInputs[0].key").value("owner")) + .andExpect( + jsonPath("$.trustedPublisherProviders[0].registrationInputs[0].description") + .value("The owner of the repository")) + .andExpect(jsonPath("$.trustedPublisherProviders[0].registrationInputs[0].optional").value(false)) + .andExpect(jsonPath("$.trustedPublisherProviders[0].registrationInputs[1].key").value("environment")) + .andExpect(jsonPath("$.trustedPublisherProviders[0].registrationInputs[1].optional").value(true)) + .andExpect(jsonPath("$.trustedPublisherProviders[1].id").value("gitlab")) + .andExpect(jsonPath("$.trustedPublisherProviders[1].name").value("GitLab CI/CD")); + } + + @Test + void getTrustedPublishingStatus_returns404_whenProviderLookupFindsFeatureDisabled() throws Exception { + var user = mockLoggedInUser(); + when(trustedPublishing.isEnabled()).thenReturn(true); + when(eclipseService.hasPublisherAgreement(user)).thenReturn(true); + when(trustedPublishing.getTrustedPublisherProviders()) + .thenThrow(new ErrorResultException("Trusted publishing is not enabled.", HttpStatus.NOT_FOUND)); + + mockMvc.perform(get("/api/-/trusted-publishing/status")) + .andExpect(status().isNotFound()) + .andExpect(jsonPath("$.error").value("Trusted publishing is not enabled.")); + } + + // --------------------------------------------------------------------------------------- + // helpers + // --------------------------------------------------------------------------------------- + + private UserData mockLoggedInUser() { + var user = new UserData(); + user.setId(1L); + user.setLoginName("test_user"); + when(users.findLoggedInUser()).thenReturn(user); + return user; + } + + private static TrustedPublisher trustedPublisher(long id) { + var namespace = new Namespace(); + namespace.setId(1L); + namespace.setName(NAMESPACE); + var extension = new Extension(); + extension.setId(2L); + extension.setName(EXTENSION); + extension.setNamespace(namespace); + + var publisher = new TrustedPublisher(); + publisher.setId(id); + publisher.setExtension(extension); + publisher.setProvider(PROVIDER); + publisher.setRegistration(REGISTRATION); + publisher.setCreatedTimestamp(LocalDateTime.of(2026, 1, 2, 3, 4, 5)); + return publisher; + } + + private static TrustedPublishingProviderSupport provider( + String id, + String name, + String url, + List registrationInputs + ) { + var provider = mock(TrustedPublishingProviderSupport.class); + when(provider.getProviderId()).thenReturn(id); + when(provider.getProviderName()).thenReturn(name); + when(provider.getProviderUrl()).thenReturn(url); + when(provider.getRegistrationInputs()).thenReturn(registrationInputs); + return provider; + } + + private static MockHttpServletRequestBuilder createRequest(String namespace, String body) { + return post("/user/namespace/{namespace}/trusted-publishing/create", namespace) + .contentType(MediaType.APPLICATION_JSON) + .content(body); + } + + private static MockHttpServletRequestBuilder tokenRequest(String body) { + return post("/api/-/trusted-publishing/token").contentType(MediaType.APPLICATION_JSON).content(body); + } + + private static String registrationBody( + String namespace, + String extension, + String provider, + Map registration + ) { + var body = new LinkedHashMap(); + body.put("namespace", namespace); + body.put("extension", extension); + body.put("provider", provider); + body.put("registration", registration); + return JsonMapper.shared().writeValueAsString(body); + } + + private static String tokenRequestBody(String namespace, String extension, String token) { + var body = new LinkedHashMap(); + body.put("namespace", namespace); + body.put("extension", extension); + body.put("token", token); + return JsonMapper.shared().writeValueAsString(body); + } +} diff --git a/server/src/test/java/org/eclipse/openvsx/trustedpublishing/TrustedPublishingConfigTest.java b/server/src/test/java/org/eclipse/openvsx/trustedpublishing/TrustedPublishingConfigTest.java new file mode 100644 index 000000000..e7ede1a55 --- /dev/null +++ b/server/src/test/java/org/eclipse/openvsx/trustedpublishing/TrustedPublishingConfigTest.java @@ -0,0 +1,172 @@ +/****************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + *****************************************************************************/ +package org.eclipse.openvsx.trustedpublishing; + +import java.time.Duration; +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.Test; +import org.springframework.boot.context.properties.bind.Bindable; +import org.springframework.boot.context.properties.bind.Binder; +import org.springframework.boot.context.properties.source.MapConfigurationPropertySource; +import org.springframework.test.util.ReflectionTestUtils; + +import org.eclipse.openvsx.trustedpublishing.gitlab.GitLabTrustedPublishingProvider; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; + +/** + * The GitLab instances are configuration rather than code, so what matters is that the public instance + * is there, that configured instances are added to it, and that a broken instance is caught at startup. + */ +class TrustedPublishingConfigTest { + + @Test + void onlyThePublicInstanceIsConfiguredByDefault() { + var instances = new TrustedPublishingConfig().getGitlab(); + + assertThat(instances).containsOnlyKeys(GitLabTrustedPublishingProvider.PROVIDER_ID); + var gitlab = instances.get(GitLabTrustedPublishingProvider.PROVIDER_ID); + assertThat(gitlab.getName()).isEqualTo("GitLab"); + assertThat(gitlab.getUrl()).isEqualTo(GitLabTrustedPublishingProvider.PROVIDER_URL); + // GitLab issues its tokens under its own base URL + assertThat(gitlab.getIssuer()).isEqualTo(GitLabTrustedPublishingProvider.PROVIDER_URL); + } + + @Test + void configuredInstanceIsAddedToThePublicOne() { + var instances = bind( + Map.of( + "ovsx.trusted-publishing.gitlab.eclipse-gitlab.name", + "Eclipse GitLab", + "ovsx.trusted-publishing.gitlab.eclipse-gitlab.url", + "https://gitlab.eclipse.org")) + .getGitlab(); + + assertThat(instances).containsOnlyKeys(GitLabTrustedPublishingProvider.PROVIDER_ID, "eclipse-gitlab"); + var eclipse = instances.get("eclipse-gitlab"); + assertThat(eclipse.getName()).isEqualTo("Eclipse GitLab"); + assertThat(eclipse.getIssuer()).isEqualTo("https://gitlab.eclipse.org"); + } + + @Test + void configuredInstanceCanRedefineThePublicOne() { + var instances = bind( + Map.of( + "ovsx.trusted-publishing.gitlab.gitlab.name", + "GitLab (staging)", + "ovsx.trusted-publishing.gitlab.gitlab.url", + "https://gitlab.staging.example", + "ovsx.trusted-publishing.gitlab.gitlab.issuer", + "https://issuer.staging.example")) + .getGitlab(); + + var gitlab = instances.get(GitLabTrustedPublishingProvider.PROVIDER_ID); + assertThat(gitlab.getName()).isEqualTo("GitLab (staging)"); + assertThat(gitlab.getUrl()).isEqualTo("https://gitlab.staging.example"); + assertThat(gitlab.getIssuer()).isEqualTo("https://issuer.staging.example"); + } + + @Test + void redefiningTheDefaultInstanceReplacesItAsAWhole() { + // only the URL is given, so the default name is gone rather than kept - and startup says so + var config = bind( + Map.of("ovsx.trusted-publishing.gitlab.gitlab.url", "https://gitlab.staging.example")); + + assertThat(config.getGitlab().get(GitLabTrustedPublishingProvider.PROVIDER_ID).getName()).isNull(); + assertThatIllegalStateException().isThrownBy(() -> enabledConfig(config).validate()) + .withMessageContaining("no name or no URL"); + } + + @Test + void issuedTokensExpireAfterFiveMinutesByDefault() { + assertThat(new TrustedPublishingConfig().getTokenExpiration()).isEqualTo(Duration.ofMinutes(5)); + } + + @Test + void tokenExpirationIsConfigurable() { + var config = bind(Map.of("ovsx.trusted-publishing.token-expiration", "PT30S")); + + assertThat(config.getTokenExpiration()).isEqualTo(Duration.ofSeconds(30)); + } + + // A token that never expires is a long-lived credential, which is what trusted publishing exists to + // avoid, so "0 means no expiry" is not on offer here the way it is for personal access tokens. + @Test + void configRejectsANonPositiveTokenExpiration() { + for (var value : List.of("PT0S", "PT-5M")) { + var config = bind(Map.of("ovsx.trusted-publishing.token-expiration", value)); + + assertThatIllegalStateException().isThrownBy(() -> enabledConfig(config).validate()) + .withMessageContaining("token-expiration must be a positive duration"); + } + } + + // ... and it is checked whether or not the feature is switched on, so a typo cannot lie in wait + @Test + void aBrokenTokenExpirationIsRejectedEvenWhileDisabled() { + var config = bind(Map.of("ovsx.trusted-publishing.token-expiration", "PT0S")); + + assertThatIllegalStateException().isThrownBy(config::validate) + .withMessageContaining("token-expiration must be a positive duration"); + } + + @Test + void enabledConfigAcceptsTheDefaultInstance() { + assertThatCode(() -> enabledConfig(new TrustedPublishingConfig()).validate()).doesNotThrowAnyException(); + } + + @Test + void enabledConfigRejectsAnInstanceTakingTheGitHubProviderId() { + var config = bind( + Map.of( + "ovsx.trusted-publishing.gitlab.github.name", + "Not GitHub", + "ovsx.trusted-publishing.gitlab.github.url", + "https://gitlab.acme.example")); + + assertThatIllegalStateException().isThrownBy(() -> enabledConfig(config).validate()) + .withMessageContaining("provider id of the GitHub provider"); + } + + @Test + void enabledConfigRejectsAMalformedInstanceUrl() { + var config = bind( + Map.of( + "ovsx.trusted-publishing.gitlab.acme-gitlab.name", + "ACME GitLab", + "ovsx.trusted-publishing.gitlab.acme-gitlab.url", + "gitlab.acme.example")); + + assertThatIllegalStateException().isThrownBy(() -> enabledConfig(config).validate()) + .withMessageContaining("malformed URL"); + } + + private static TrustedPublishingConfig bind(Map properties) { + return new Binder(new MapConfigurationPropertySource(properties)) + .bind("ovsx.trusted-publishing", Bindable.ofInstance(new TrustedPublishingConfig())) + .orElseGet(TrustedPublishingConfig::new); + } + + // @Value is not resolved for a hand-built instance, so the scalar settings are set directly + private static TrustedPublishingConfig enabledConfig(TrustedPublishingConfig config) { + ReflectionTestUtils.setField(config, "enabled", true); + ReflectionTestUtils.setField(config, "audience", "https://open-vsx.org"); + ReflectionTestUtils.setField(config, "forbiddenJwtHeaders", List.of("x5u")); + ReflectionTestUtils.setField(config, "activeProviders", List.of("github")); + return config; + } +} diff --git a/server/src/test/java/org/eclipse/openvsx/trustedpublishing/TrustedPublishingServiceTest.java b/server/src/test/java/org/eclipse/openvsx/trustedpublishing/TrustedPublishingServiceTest.java index d39d749a9..2a5f82f11 100644 --- a/server/src/test/java/org/eclipse/openvsx/trustedpublishing/TrustedPublishingServiceTest.java +++ b/server/src/test/java/org/eclipse/openvsx/trustedpublishing/TrustedPublishingServiceTest.java @@ -12,6 +12,7 @@ *****************************************************************************/ package org.eclipse.openvsx.trustedpublishing; +import java.util.LinkedHashMap; import java.util.List; import jakarta.persistence.EntityManager; @@ -28,6 +29,7 @@ import org.eclipse.openvsx.entities.TrustedPublisher; import org.eclipse.openvsx.entities.UserData; import org.eclipse.openvsx.repositories.RepositoryService; +import org.eclipse.openvsx.trustedpublishing.TrustedPublishingConfig.GitLabInstance; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.when; @@ -91,6 +93,23 @@ void listsEveryRegistrationButOffersOnlyUntakenActiveExtensions() { assertThat(result.registrableExtensions()).containsExactly("free"); } + // The status endpoint hands this list straight to the client, so an unordered map would reshuffle + // the providers offered in the registration dialog on every restart. + @Test + void offersProvidersInAStableOrderGitHubFirst() { + when(config.isEnabled()).thenReturn(true); + when(config.getActiveProviders()).thenReturn(List.of("github", "gitlab", "eclipse-gitlab")); + var instances = new LinkedHashMap(); + instances.put("gitlab", new GitLabInstance("GitLab", "https://gitlab.com")); + instances.put("eclipse-gitlab", new GitLabInstance("Eclipse GitLab", "https://gitlab.eclipse.org")); + when(config.getGitlab()).thenReturn(instances); + + var service = new TrustedPublishingService(config, repositories, tokens, entityManager); + + assertThat(service.getTrustedPublisherProviders().keySet()) + .containsExactly("github", "gitlab", "eclipse-gitlab"); + } + private Extension extension(long id, String name) { var extension = new Extension(); extension.setId(id); diff --git a/server/src/test/java/org/eclipse/openvsx/trustedpublishing/gitlab/GitLabTrustedPublishingProviderTest.java b/server/src/test/java/org/eclipse/openvsx/trustedpublishing/gitlab/GitLabTrustedPublishingProviderTest.java index 992c380e8..bb848ec23 100644 --- a/server/src/test/java/org/eclipse/openvsx/trustedpublishing/gitlab/GitLabTrustedPublishingProviderTest.java +++ b/server/src/test/java/org/eclipse/openvsx/trustedpublishing/gitlab/GitLabTrustedPublishingProviderTest.java @@ -26,6 +26,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @ExtendWith(SpringExtension.class) @@ -57,7 +58,18 @@ private static Map registeredClaims() { } private static GitLabTrustedPublishingProvider newProvider(TrustedPublishingConfig config) { - return new GitLabTrustedPublishingProvider(config) { + return newProvider( + config, + GitLabTrustedPublishingProvider.PROVIDER_ID, + GitLabTrustedPublishingProvider.PROVIDER_URL); + } + + private static GitLabTrustedPublishingProvider newProvider( + TrustedPublishingConfig config, + String providerId, + String providerUrl + ) { + return new GitLabTrustedPublishingProvider(config, providerId, "GitLab", providerUrl, providerUrl) { @Override protected Map resolve(String projectPath) { return Map.of( @@ -151,6 +163,42 @@ void pinnedEnvironment() { assertTrue(gl.matches(registeredClaims(), token)); } + @Test + void selfHostedInstanceIsAddressedByItsOwnUrl() throws Exception { + GitLabTrustedPublishingProvider gl = newProvider(config, "acme-gitlab", "https://gitlab.acme.example"); + Map data = gl.extractRequest( + Map.of( + "namespace", + "gitlab-org", + "project", + "gitlab", + "workflow", + ".gitlab-ci.yml")); + assertEquals("gitlab.acme.example/gitlab-org/gitlab//.gitlab-ci.yml", data.get("ci_config_ref_uri")); + assertEquals("acme-gitlab", gl.getProviderId()); + } + + @Test + void instanceServedUnderARelativeUrlRootKeepsItsPath() throws Exception { + GitLabTrustedPublishingProvider gl = newProvider(config, "acme-gitlab", "https://acme.example/gitlab/"); + Map data = gl.extractRequest( + Map.of( + "namespace", + "gitlab-org", + "project", + "gitlab", + "workflow", + ".gitlab-ci.yml")); + assertEquals("acme.example/gitlab/gitlab-org/gitlab//.gitlab-ci.yml", data.get("ci_config_ref_uri")); + } + + @Test + void malformedInstanceUrlIsRejected() { + assertThrows( + IllegalArgumentException.class, + () -> newProvider(config, "acme-gitlab", "not-a-url")); + } + @TestConfiguration static class TestConfig { @Bean diff --git a/webui/CHANGELOG.md b/webui/CHANGELOG.md index c18d9f6b6..bc96f33a2 100644 --- a/webui/CHANGELOG.md +++ b/webui/CHANGELOG.md @@ -11,6 +11,7 @@ This change log covers only the frontend library (webui) of Open VSX. - Add a "Data Consistency" page to the admin dashboard (#1622): a live overview of every registered consistency check's finding count, with actions to refresh it and to fix findings one at a time or all at once - Show a "Namespace not verified" state on an extension card when it can't be activated because its namespace already exists in a referenced external gallery and hasn't been verified, in both the "My Extensions" and namespace member extension lists. The card keeps its colour and takes a warning-toned frame and icon, since this is the publisher's to fix rather than an extension that is simply switched off - Show a warning notice with a claim action wherever an unverified namespace is holding something back — the extension settings page when the extension has a namespace ownership conflict, and the namespace settings page for any unverified namespace — making clear the namespace must be claimed (verified) first. The action is the deployment's configured `elements.claimNamespace`, falling back to the namespace access documentation when none is configured. The admin dashboard's extension and namespace views show the same explanation without the claim action, since claiming is the publisher's action to take, not an admin's on someone else's behalf +- Mark a version that was published through a trusted publishing workflow with an icon next to "Published by" on the extension detail page, linking to the deployment's trusted publishing documentation. The default deployment points that link at the [Trusted Publishing](https://github.com/eclipse-openvsx/openvsx/wiki/Trusted-Publishing) wiki page ### Changed diff --git a/webui/src/default/page-settings.tsx b/webui/src/default/page-settings.tsx index 44f667991..7115edbc1 100644 --- a/webui/src/default/page-settings.tsx +++ b/webui/src/default/page-settings.tsx @@ -206,7 +206,7 @@ export default function createPageSettings(prefersDarkMode: boolean, serverUrl: extensionDefaultIcon: '/default-icon.png', namespaceAccessInfo: 'https://github.com/eclipse/openvsx/wiki/Namespace-Access', publisherAgreement: createAbsoluteURL([serverUrl, 'documents', 'publisher-agreement.md']), - trustedPublishing: 'https://repos.openssf.org/trusted-publishers-for-all-package-repositories' + trustedPublishing: 'https://github.com/eclipse-openvsx/openvsx/wiki/Trusted-Publishing' } }; } diff --git a/webui/src/extension-registry-types.ts b/webui/src/extension-registry-types.ts index c03a7bcec..377cc70df 100644 --- a/webui/src/extension-registry-types.ts +++ b/webui/src/extension-registry-types.ts @@ -75,6 +75,8 @@ export interface Extension { targetPlatform: string; preRelease?: boolean; publishedBy: UserData; + /** True when this version was published through a trusted publishing workflow. */ + publishedWithTrustedPublishing?: boolean; verified: boolean; // key: version, value: url allVersions: { [version: string]: UrlString }; diff --git a/webui/src/pages/extension-detail/extension-detail.tsx b/webui/src/pages/extension-detail/extension-detail.tsx index a53eed728..41155370c 100644 --- a/webui/src/pages/extension-detail/extension-detail.tsx +++ b/webui/src/pages/extension-detail/extension-detail.tsx @@ -38,6 +38,7 @@ import { ExtensionDetailChanges } from './extension-detail-changes'; import { ExtensionDetailReviews } from './extension-detail-reviews'; import { ExtensionDetailRoutes } from './extension-detail-routes'; +import { TrustedPublishingIcon } from './trusted-publishing-icon'; import { useExtensionDetail } from './use-extension-details'; import { KbdKey } from '../../components/kbd-key'; import { useShortcut } from '../../hooks/use-shortcut'; @@ -187,7 +188,7 @@ const LicenseLink: FunctionComponent<{ return <>{extension.license || 'Unlicensed'}; }; -const ExtensionHeaderInfo: FunctionComponent<{ +export const ExtensionHeaderInfo: FunctionComponent<{ extension: Extension; headerTextColor: string; }> = ({ extension, headerTextColor }) => { @@ -248,6 +249,12 @@ const ExtensionHeaderInfo: FunctionComponent<{ Published by  + {extension.publishedWithTrustedPublishing && ( + <> + + + + )} diff --git a/webui/src/pages/extension-detail/trusted-publishing-icon.tsx b/webui/src/pages/extension-detail/trusted-publishing-icon.tsx new file mode 100644 index 000000000..64ff5414d --- /dev/null +++ b/webui/src/pages/extension-detail/trusted-publishing-icon.tsx @@ -0,0 +1,56 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ + +import { FunctionComponent, useContext } from 'react'; +import { Link } from '@mui/material'; +import { styled } from '@mui/material/styles'; +import { MainContext } from '../../context'; +import VerifiedIcon from '@mui/icons-material/Verified'; + +const IconLink = styled(Link)(({ theme }) => ({ + display: 'flex', + alignItems: 'center', + marginLeft: theme.spacing(0.5) +})); + +const IconBadge = styled('span')(({ theme }) => ({ + display: 'flex', + alignItems: 'center', + marginLeft: theme.spacing(0.5) +})); + +/** + * Marks a version that was published from a trusted publishing workflow instead of with a + * personal access token. Whether a version qualifies is the caller's to decide, since the + * header pairs the icon with a divider. + */ +export const TrustedPublishingIcon: FunctionComponent<{ + color: string; +}> = ({ color }) => { + const { pageSettings } = useContext(MainContext); + + const title = 'Published via trusted publishing'; + const url = pageSettings.urls.trustedPublishing; + const icon = ; + + // a plain badge when this instance configures no documentation URL to link to + return url ? ( + + {icon} + + ) : ( + + {icon} + + ); +}; diff --git a/webui/test/unit/pages/extension-detail/extension-detail.spec.tsx b/webui/test/unit/pages/extension-detail/extension-detail.spec.tsx new file mode 100644 index 000000000..75a9e4053 --- /dev/null +++ b/webui/test/unit/pages/extension-detail/extension-detail.spec.tsx @@ -0,0 +1,70 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ + +import { describe, it, expect } from 'vitest'; +import { screen } from '@testing-library/react'; +import { ExtensionHeaderInfo } from '../../../../src/pages/extension-detail/extension-detail'; +import { Extension } from '../../../../src/extension-registry-types'; +import { PageSettings } from '../../../../src/page-settings'; +import { renderWithProviders } from '../../support/test-providers'; + +const TRUSTED_PUBLISHING_TITLE = 'Published via trusted publishing'; + +const extension = (overrides: Partial = {}): Extension => + ({ + name: 'bar', + namespace: 'foo', + namespaceDisplayName: 'Foo', + version: '1.0.0', + displayName: 'Bar Tools', + files: {}, + downloadCount: 0, + reviewCount: 0, + deprecated: false, + verified: true, + publishedBy: { loginName: 'test_user', homepage: 'https://example.com/test_user' }, + ...overrides + }) as unknown as Extension; + +const renderHeaderInfo = (overrides: Partial = {}) => + renderWithProviders(, { + mainContext: { + pageSettings: { + elements: {}, + urls: { + namespaceAccessInfo: 'https://example.com/namespace-access', + trustedPublishing: 'https://example.com/trusted-publishing' + } + } as PageSettings + } + }); + +describe('ExtensionHeaderInfo', () => { + it('marks a version published through trusted publishing', () => { + renderHeaderInfo({ publishedWithTrustedPublishing: true }); + + expect(screen.getByLabelText(TRUSTED_PUBLISHING_TITLE)).toBeInTheDocument(); + }); + + it('leaves a version published with an access token unmarked', () => { + renderHeaderInfo({ publishedWithTrustedPublishing: false }); + + expect(screen.queryByLabelText(TRUSTED_PUBLISHING_TITLE)).not.toBeInTheDocument(); + }); + + it('leaves the mark off when the registry does not report how a version was published', () => { + renderHeaderInfo(); + + expect(screen.queryByLabelText(TRUSTED_PUBLISHING_TITLE)).not.toBeInTheDocument(); + }); +}); diff --git a/webui/test/unit/pages/extension-detail/trusted-publishing-icon.spec.tsx b/webui/test/unit/pages/extension-detail/trusted-publishing-icon.spec.tsx new file mode 100644 index 000000000..21a4fbb31 --- /dev/null +++ b/webui/test/unit/pages/extension-detail/trusted-publishing-icon.spec.tsx @@ -0,0 +1,45 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + ********************************************************************************/ + +import { describe, it, expect } from 'vitest'; +import { screen } from '@testing-library/react'; +import { TrustedPublishingIcon } from '../../../../src/pages/extension-detail/trusted-publishing-icon'; +import { PageSettings } from '../../../../src/page-settings'; +import { renderWithProviders } from '../../support/test-providers'; + +const TITLE = 'Published via trusted publishing'; + +const pageSettings = (trustedPublishing?: string) => ({ elements: {}, urls: { trustedPublishing } }) as PageSettings; + +describe('TrustedPublishingIcon', () => { + it('links to the documentation when the instance configures a URL for it', () => { + renderWithProviders(, { + mainContext: { pageSettings: pageSettings('https://example.com/trusted-publishing') } + }); + + const link = screen.getByLabelText(TITLE); + expect(link.tagName).toBe('A'); + expect(link).toHaveAttribute('href', 'https://example.com/trusted-publishing'); + expect(link).toHaveAttribute('target', '_blank'); + }); + + it('still shows the icon when no documentation URL is configured', () => { + renderWithProviders(, { + mainContext: { pageSettings: pageSettings() } + }); + + const icon = screen.getByLabelText(TITLE); + expect(icon).toBeInTheDocument(); + expect(icon.tagName).not.toBe('A'); + }); +});