Skip to content

Commit 435233a

Browse files
netominetomi
authored
Ensure to retrieve and display up-to-date publisher agreement status (#1445)
* use the appropriate api call to get up-to-date information about publisher agreements * fallback to public profile if no token is available, small refactorings and fixes, adjust and add more unit tests * use buildApiUrl method * only issue a warning without stacktrace in case refreshing the token failed due to a 400 response
1 parent 4146fd8 commit 435233a

File tree

9 files changed

+218
-82
lines changed

9 files changed

+218
-82
lines changed

‎server/src/main/java/org/eclipse/openvsx/UserAPI.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ public UserJson getUserData() {
121121
json.setRole(user.getRole());
122122
json.setTokensUrl(createApiUrl(serverUrl, "user", "tokens"));
123123
json.setCreateTokenUrl(createApiUrl(serverUrl, "user", "token", "create"));
124-
eclipse.enrichUserJson(json, user);
124+
eclipse.enrichUserJsonWithPublisherAgreement(json, user);
125125
return json;
126126
}
127127

‎server/src/main/java/org/eclipse/openvsx/eclipse/EclipseProfile.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
import java.io.IOException;
2121
import java.util.List;
22+
import java.util.Optional;
2223

2324
public class EclipseProfile {
2425

@@ -129,6 +130,14 @@ public void setPublisherAgreements(PublisherAgreements publisherAgreements) {
129130
this.publisherAgreements = publisherAgreements;
130131
}
131132

133+
public Optional<PublisherAgreement> getOpenVsxPublisherAgreement() {
134+
if (publisherAgreements != null && publisherAgreements.getOpenVsx() != null) {
135+
return Optional.of(publisherAgreements.getOpenVsx());
136+
} else {
137+
return Optional.empty();
138+
}
139+
}
140+
132141
public static class PublisherAgreements {
133142

134143
@JsonProperty("open-vsx")
@@ -152,7 +161,7 @@ public PublisherAgreements deserialize(JsonParser p, DeserializationContext ctxt
152161
var list = p.getCodec().readValue(p, TYPE_LIST_AGREEMENT);
153162
var result = new PublisherAgreements();
154163
if (!list.isEmpty())
155-
result.openVsx = list.get(0);
164+
result.openVsx = list.getFirst();
156165
return result;
157166
}
158167
return p.getCodec().readValue(p, PublisherAgreements.class);

‎server/src/main/java/org/eclipse/openvsx/eclipse/EclipseService.java‎

Lines changed: 74 additions & 46 deletions
Large diffs are not rendered by default.

‎server/src/main/java/org/eclipse/openvsx/eclipse/TokenService.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.springframework.transaction.support.TransactionTemplate;
3030
import org.springframework.util.LinkedMultiValueMap;
3131
import org.springframework.util.MultiValueMap;
32+
import org.springframework.web.client.HttpClientErrorException;
3233
import org.springframework.web.client.RestClientException;
3334
import org.springframework.web.client.RestTemplate;
3435

@@ -146,6 +147,9 @@ private Pair<OAuth2AccessToken, OAuth2RefreshToken> refreshEclipseToken(AuthToke
146147
var newToken = new OAuth2AccessToken(TokenType.BEARER, newTokenValue, issuedAt, expiresAt);
147148
var newRefreshToken = new OAuth2RefreshToken(newRefreshTokenValue, issuedAt);
148149
return Pair.of(newToken, newRefreshToken);
150+
} catch (HttpClientErrorException.BadRequest exc) {
151+
// keycloak sends a 400 status response if the refresh call failed
152+
logger.warn("Eclipse token could not be refreshed: {}", exc.getMessage());
149153
} catch (RestClientException exc) {
150154
logger.error("Post request failed with URL: {}", tokenUri, exc);
151155
} catch (JsonProcessingException exc) {

‎server/src/main/java/org/eclipse/openvsx/json/UserJson.java‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,9 @@ public static class PublisherAgreement {
147147
/* 'none' | 'signed' | 'outdated' */
148148
private String status;
149149

150+
@Schema(hidden = true)
151+
private String version;
152+
150153
private String timestamp;
151154

152155
public String getStatus() {
@@ -157,6 +160,14 @@ public void setStatus(String status) {
157160
this.status = status;
158161
}
159162

163+
public String getVersion() {
164+
return version;
165+
}
166+
167+
public void setVersion(String version) {
168+
this.version = version;
169+
}
170+
160171
public String getTimestamp() {
161172
return timestamp;
162173
}
@@ -170,12 +181,14 @@ public boolean equals(Object o) {
170181
if (this == o) return true;
171182
if (o == null || getClass() != o.getClass()) return false;
172183
PublisherAgreement that = (PublisherAgreement) o;
173-
return Objects.equals(status, that.status) && Objects.equals(timestamp, that.timestamp);
184+
return Objects.equals(status, that.status) &&
185+
Objects.equals(version, that.version) &&
186+
Objects.equals(timestamp, that.timestamp);
174187
}
175188

176189
@Override
177190
public int hashCode() {
178-
return Objects.hash(status, timestamp);
191+
return Objects.hash(status, version, timestamp);
179192
}
180193
}
181194

‎server/src/main/java/org/eclipse/openvsx/util/HttpHeadersUtil.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
package org.eclipse.openvsx.util;
22

33
import org.springframework.http.HttpHeaders;
4+
import org.springframework.http.MediaType;
45
import org.springframework.web.context.request.RequestContextHolder;
56
import org.springframework.web.context.request.ServletRequestAttributes;
67

8+
import java.util.List;
9+
710
public class HttpHeadersUtil {
811
private HttpHeadersUtil() {
912
}
@@ -20,9 +23,14 @@ public static HttpHeaders getForwardedHeaders() {
2023
headers.add(header, request.getHeader(header));
2124
}
2225

23-
} catch (IllegalStateException e) {
24-
}
26+
} catch (IllegalStateException _) {}
2527
headers.remove(HttpHeaders.HOST);
2628
return headers;
2729
}
30+
31+
public static HttpHeaders getAcceptJsonHeaders() {
32+
var headers = new HttpHeaders();
33+
headers.setAccept(List.of(MediaType.APPLICATION_JSON));
34+
return headers;
35+
}
2836
}

‎server/src/test/java/org/eclipse/openvsx/eclipse/EclipseServiceTest.java‎

Lines changed: 91 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@
6262
})
6363
class EclipseServiceTest {
6464

65+
private static final String PUBLIC_PROFILE_URL = "https://test.openvsx.eclipse.org/account/profile/{personId}";
66+
private static final String PUBLISHER_AGREEMENT_URL = "https://test.openvsx.eclipse.org/openvsx/publisher_agreement/{personId}";
67+
6568
@MockitoBean
6669
RepositoryService repositories;
6770

@@ -83,9 +86,15 @@ void setup() {
8386

8487
@Test
8588
void testGetPublicProfile() throws Exception {
86-
var urlTemplate = "https://test.openvsx.eclipse.org/account/profile/{personId}";
87-
Mockito.when(restTemplate.exchange(eq(urlTemplate), eq(HttpMethod.GET), any(HttpEntity.class), eq(String.class), eq(Map.of("personId", "test"))))
88-
.thenReturn(mockProfileResponse());
89+
Mockito.when(
90+
restTemplate.exchange(
91+
eq(PUBLIC_PROFILE_URL),
92+
eq(HttpMethod.GET),
93+
any(HttpEntity.class),
94+
eq(String.class),
95+
eq(Map.of("personId", "test"))
96+
)
97+
).thenReturn(mockProfileResponse());
8998

9099
var profile = eclipse.getPublicProfile("test");
91100

@@ -118,9 +127,15 @@ void testGetPublisherAgreement() throws Exception {
118127
var user = mockUser();
119128
user.setEclipsePersonId("test");
120129

121-
var urlTemplate = "https://test.openvsx.eclipse.org/openvsx/publisher_agreement/{personId}";
122-
Mockito.when(restTemplate.exchange(eq(urlTemplate), eq(HttpMethod.GET), any(HttpEntity.class), eq(String.class), eq(Map.of("personId", "test"))))
123-
.thenReturn(mockAgreementResponse());
130+
Mockito.when(
131+
restTemplate.exchange(
132+
eq(PUBLISHER_AGREEMENT_URL),
133+
eq(HttpMethod.GET),
134+
any(HttpEntity.class),
135+
eq(String.class),
136+
eq(Map.of("personId", "test"))
137+
)
138+
).thenReturn(mockAgreementResponse());
124139

125140
var agreement = eclipse.getPublisherAgreement(user);
126141
assertThat(agreement).isNotNull();
@@ -131,13 +146,42 @@ void testGetPublisherAgreement() throws Exception {
131146
}
132147

133148
@Test
134-
void testCheckPublisherAgreementOutdated() throws Exception {
149+
void testCheckPublisherOutdatedAgreement() throws Exception {
135150
var user = mockUser();
136151
user.setEclipsePersonId("test");
137152

138-
var urlTemplate = "https://test.openvsx.eclipse.org/account/profile/{personId}";
139-
Mockito.when(restTemplate.exchange(eq(urlTemplate), eq(HttpMethod.GET), any(HttpEntity.class), eq(String.class), eq(Map.of("personId", "test"))))
140-
.thenReturn(mockOutdatedProfileResponse());
153+
Mockito.when(
154+
restTemplate.exchange(
155+
eq(PUBLISHER_AGREEMENT_URL),
156+
eq(HttpMethod.GET),
157+
any(HttpEntity.class),
158+
eq(String.class),
159+
eq(Map.of("personId", "test"))
160+
)
161+
).thenReturn(mockOutdatedAgreementResponse());
162+
163+
try {
164+
eclipse.checkPublisherAgreement(user);
165+
fail("Expected an ErrorResultException");
166+
} catch(ErrorResultException exc) {
167+
assertThat(exc.getMessage()).isEqualTo("Your Publisher Agreement with the Eclipse Foundation is outdated (version 0.1). The current version is 1.1.");
168+
}
169+
}
170+
171+
@Test
172+
void testCheckPublisherOutdatedAgreementNoToken() throws Exception {
173+
var user = mockUserNoToken();
174+
user.setEclipsePersonId("test");
175+
176+
Mockito.when(
177+
restTemplate.exchange(
178+
eq(PUBLIC_PROFILE_URL),
179+
eq(HttpMethod.GET),
180+
any(HttpEntity.class),
181+
eq(String.class),
182+
eq(Map.of("personId", "test"))
183+
)
184+
).thenReturn(mockOutdatedProfileResponse());
141185

142186
try {
143187
eclipse.checkPublisherAgreement(user);
@@ -152,21 +196,33 @@ void testCheckPublisherAgreementAllowed() throws Exception {
152196
var user = mockUser();
153197
user.setEclipsePersonId("test");
154198

155-
var urlTemplate = "https://test.openvsx.eclipse.org/account/profile/{personId}";
156-
Mockito.when(restTemplate.exchange(eq(urlTemplate), eq(HttpMethod.GET), any(HttpEntity.class), eq(String.class), eq(Map.of("personId", "test"))))
157-
.thenReturn(mockAllowedProfileResponse());
199+
Mockito.when(
200+
restTemplate.exchange(
201+
eq(PUBLISHER_AGREEMENT_URL),
202+
eq(HttpMethod.GET),
203+
any(HttpEntity.class),
204+
eq(String.class),
205+
eq(Map.of("personId", "test"))
206+
)
207+
).thenReturn(mockAgreementResponse());
158208

159209
eclipse.checkPublisherAgreement(user);
160210
}
161211

162212
@Test
163-
void testCheckPublisherAgreement() throws Exception {
164-
var user = mockUser();
213+
void testCheckPublisherAgreementAllowedNoToken() throws Exception {
214+
var user = mockUserNoToken();
165215
user.setEclipsePersonId("test");
166216

167-
var urlTemplate = "https://test.openvsx.eclipse.org/account/profile/{personId}";
168-
Mockito.when(restTemplate.exchange(eq(urlTemplate), eq(HttpMethod.GET), any(HttpEntity.class), eq(String.class), eq(Map.of("personId", "test"))))
169-
.thenReturn(mockProfileResponse());
217+
Mockito.when(
218+
restTemplate.exchange(
219+
eq(PUBLIC_PROFILE_URL),
220+
eq(HttpMethod.GET),
221+
any(HttpEntity.class),
222+
eq(String.class),
223+
eq(Map.of("personId", "test"))
224+
)
225+
).thenReturn(mockAllowedProfileResponse());
170226

171227
eclipse.checkPublisherAgreement(user);
172228
}
@@ -284,6 +340,15 @@ private UserData mockUser() {
284340
return user;
285341
}
286342

343+
private UserData mockUserNoToken() {
344+
var user = new UserData();
345+
user.setLoginName("test");
346+
user.setProvider("github");
347+
Mockito.when(tokens.getActiveEclipseToken(user))
348+
.thenReturn(null);
349+
return user;
350+
}
351+
287352
private ResponseEntity<String> mockProfileResponse() throws IOException {
288353
try (var stream = getClass().getResourceAsStream("profile-response.json")) {
289354
var json = CharStreams.toString(new InputStreamReader(stream));
@@ -311,7 +376,14 @@ private ResponseEntity<String> mockAgreementResponse() throws IOException {
311376
return new ResponseEntity<>(json, HttpStatus.OK);
312377
}
313378
}
314-
379+
380+
private ResponseEntity<String> mockOutdatedAgreementResponse() throws IOException {
381+
try (var stream = getClass().getResourceAsStream("publisher-agreement-outdated-response.json")) {
382+
var json = CharStreams.toString(new InputStreamReader(stream));
383+
return new ResponseEntity<>(json, HttpStatus.OK);
384+
}
385+
}
386+
315387
@TestConfiguration
316388
static class TestConfig {
317389
@Bean

‎server/src/test/resources/org/eclipse/openvsx/eclipse/publisher-agreement-outdated-response.json‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"PersonID": "test",
3+
"DocumentID": "abcd",
4+
"Version": "0.1",
5+
"EffectiveDate": "2020-10-09 05:10:32",
6+
"ReceivedDate": "2020-10-09",
7+
"ExpirationDate": null,
8+
"ScannedDocumentBLOB": null,
9+
"ScannedDocumentMime": "application/json",
10+
"ScannedDocumentBytes": "117",
11+
"ScannedDocumentFileName": "openvsx-publisher-agreement.json"
12+
}

‎server/src/test/resources/org/eclipse/openvsx/eclipse/publisher-agreement-response.json‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,5 @@
88
"ScannedDocumentBLOB": null,
99
"ScannedDocumentMime": "application/json",
1010
"ScannedDocumentBytes": "117",
11-
"ScannedDocumentFileName": "openvsx-publisher-agreement.json",
12-
"SysDocument": {
13-
"DocumentID": "abcd",
14-
"Version": "1.1",
15-
"DocumentBLOB": null,
16-
"IsActive": "1",
17-
"Description": "Publisher Agreement - open-vsx.org",
18-
"Comments": "Generated by api.eclipse.org/openvsx/publisher_agreement",
19-
"Type": "IN",
20-
"ContentType": ""
21-
}
11+
"ScannedDocumentFileName": "openvsx-publisher-agreement.json"
2212
}

0 commit comments

Comments
 (0)