Skip to content

Commit 15991f6

Browse files
amvanbarenamvanbaren
authored
Merge pull request #1132 from amvanbaren/remove-github-token
Remove github token
2 parents 299e36d + 1fc7df6 commit 15991f6

File tree

11 files changed

+86
-156
lines changed

11 files changed

+86
-156
lines changed

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import org.eclipse.openvsx.entities.AuthToken;
2222
import org.eclipse.openvsx.entities.UserData;
2323
import org.eclipse.openvsx.json.UserJson;
24-
import org.eclipse.openvsx.security.TokenService;
2524
import org.eclipse.openvsx.util.ErrorResultException;
2625
import org.eclipse.openvsx.util.TimeUtil;
2726
import org.eclipse.openvsx.util.UrlUtil;
@@ -455,7 +454,7 @@ private void checkApiUrl() {
455454
}
456455

457456
private AuthToken checkEclipseToken(UserData user) {
458-
var eclipseToken = tokens.getActiveToken(user, "eclipse");
457+
var eclipseToken = tokens.getActiveEclipseToken(user);
459458
if (eclipseToken == null || StringUtils.isEmpty(eclipseToken.accessToken())) {
460459
throw new ErrorResultException("Authorization by Eclipse required.", HttpStatus.FORBIDDEN);
461460
}

server/src/main/java/org/eclipse/openvsx/security/TokenService.java renamed to server/src/main/java/org/eclipse/openvsx/eclipse/TokenService.java

Lines changed: 46 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
* SPDX-License-Identifier: EPL-2.0
99
********************************************************************************/
10-
package org.eclipse.openvsx.security;
10+
package org.eclipse.openvsx.eclipse;
1111

1212
import com.fasterxml.jackson.core.JsonProcessingException;
1313
import com.fasterxml.jackson.databind.ObjectMapper;
@@ -16,6 +16,7 @@
1616
import org.eclipse.openvsx.entities.UserData;
1717
import org.slf4j.Logger;
1818
import org.slf4j.LoggerFactory;
19+
import org.springframework.beans.factory.annotation.Autowired;
1920
import org.springframework.data.util.Pair;
2021
import org.springframework.http.HttpEntity;
2122
import org.springframework.http.HttpHeaders;
@@ -28,10 +29,10 @@
2829
import org.springframework.transaction.support.TransactionTemplate;
2930
import org.springframework.web.client.RestClientException;
3031
import org.springframework.web.client.RestTemplate;
31-
import org.springframework.beans.factory.annotation.Autowired;
3232

3333
import java.time.Instant;
34-
import java.util.Arrays;
34+
import java.util.List;
35+
import java.util.Optional;
3536

3637
@Component
3738
public class TokenService {
@@ -52,105 +53,51 @@ public TokenService(
5253
this.clientRegistrationRepository = clientRegistrationRepository;
5354
}
5455

55-
private boolean isEnabled() {
56-
return clientRegistrationRepository != null;
57-
}
58-
59-
public AuthToken updateTokens(long userId, String registrationId, OAuth2AccessToken accessToken,
60-
OAuth2RefreshToken refreshToken) {
61-
var userData = isEnabled() ? entityManager.find(UserData.class, userId) : null;
62-
if (userData == null) {
63-
return null;
64-
}
65-
66-
switch (registrationId) {
67-
case "github": {
68-
if (accessToken == null) {
69-
return updateGitHubToken(userData, null);
70-
}
71-
72-
var token = new AuthToken(
73-
accessToken.getTokenValue(),
74-
accessToken.getIssuedAt(),
75-
accessToken.getExpiresAt(),
76-
accessToken.getScopes(),
77-
null,
78-
null
79-
);
80-
return updateGitHubToken(userData, token);
81-
}
82-
83-
case "eclipse": {
84-
if (accessToken == null) {
85-
return updateEclipseToken(userData, null);
86-
}
87-
88-
String refresh = null;
89-
Instant refreshExpiresAt = null;
90-
if (refreshToken != null) {
91-
refresh = refreshToken.getTokenValue();
92-
refreshExpiresAt = refreshToken.getExpiresAt();
93-
}
94-
95-
var token = new AuthToken(
96-
accessToken.getTokenValue(),
97-
accessToken.getIssuedAt(),
98-
accessToken.getExpiresAt(),
99-
accessToken.getScopes(),
100-
refresh,
101-
refreshExpiresAt
102-
);
103-
104-
return updateEclipseToken(userData, token);
105-
}
106-
}
107-
return null;
108-
}
109-
110-
private AuthToken updateGitHubToken(UserData userData, AuthToken token) {
111-
return transactions.execute(status -> {
112-
userData.setGithubToken(token);
113-
entityManager.merge(userData);
114-
return token;
115-
});
116-
}
117-
118-
private AuthToken updateEclipseToken(UserData userData, AuthToken token) {
56+
public AuthToken updateEclipseToken(long userId, OAuth2AccessToken accessToken, OAuth2RefreshToken refreshToken) {
57+
var token = toAuthToken(accessToken, refreshToken);
11958
return transactions.execute(status -> {
59+
var userData = entityManager.find(UserData.class, userId);
12060
userData.setEclipseToken(token);
121-
entityManager.merge(userData);
12261
return token;
12362
});
12463
}
12564

126-
public AuthToken getActiveToken(UserData userData, String registrationId) {
127-
if(!isEnabled()) {
65+
private AuthToken toAuthToken(OAuth2AccessToken accessToken, OAuth2RefreshToken refreshToken) {
66+
if(accessToken == null) {
12867
return null;
12968
}
13069

131-
switch (registrationId) {
132-
case "github": {
133-
return userData.getGithubToken();
134-
}
70+
String refresh = null;
71+
Instant refreshExpiresAt = null;
72+
if (refreshToken != null) {
73+
refresh = refreshToken.getTokenValue();
74+
refreshExpiresAt = refreshToken.getExpiresAt();
75+
}
76+
77+
return new AuthToken(
78+
accessToken.getTokenValue(),
79+
accessToken.getIssuedAt(),
80+
accessToken.getExpiresAt(),
81+
accessToken.getScopes(),
82+
refresh,
83+
refreshExpiresAt
84+
);
85+
}
13586

136-
case "eclipse": {
137-
var token = userData.getEclipseToken();
138-
if (token != null && isExpired(token.expiresAt())) {
139-
OAuth2AccessToken newAccessToken = null;
140-
OAuth2RefreshToken newRefreshToken = null;
141-
var newTokens = refreshEclipseToken(token);
142-
if (newTokens != null) {
143-
newAccessToken = newTokens.getFirst();
144-
newRefreshToken = newTokens.getSecond();
145-
}
146-
147-
return updateTokens(userData.getId(), "eclipse", newAccessToken, newRefreshToken);
148-
}
149-
return token;
87+
public AuthToken getActiveEclipseToken(UserData userData) {
88+
var token = userData.getEclipseToken();
89+
if (token != null && isExpired(token.expiresAt())) {
90+
OAuth2AccessToken newAccessToken = null;
91+
OAuth2RefreshToken newRefreshToken = null;
92+
var newTokens = refreshEclipseToken(token);
93+
if (newTokens != null) {
94+
newAccessToken = newTokens.getFirst();
95+
newRefreshToken = newTokens.getSecond();
15096
}
151-
}
15297

153-
return null;
98+
return updateEclipseToken(userData.getId(), newAccessToken, newRefreshToken);
99+
}
100+
return token;
154101
}
155102

156103
private boolean isExpired(Instant instant) {
@@ -162,12 +109,17 @@ private Pair<OAuth2AccessToken, OAuth2RefreshToken> refreshEclipseToken(AuthToke
162109
return null;
163110
}
164111

165-
var reg = clientRegistrationRepository.findByRegistrationId("eclipse");
112+
var reg = Optional.ofNullable(clientRegistrationRepository).map(repo -> repo.findByRegistrationId("eclipse")).orElse(null);
113+
if(reg == null) {
114+
logger.error("Eclipse client not registered");
115+
return null;
116+
}
117+
166118
var tokenUri = reg.getProviderDetails().getTokenUri();
167119

168120
var headers = new HttpHeaders();
169121
headers.setContentType(MediaType.APPLICATION_JSON);
170-
headers.setAccept(Arrays.asList(MediaType.APPLICATION_JSON));
122+
headers.setAccept(List.of(MediaType.APPLICATION_JSON));
171123

172124
var objectMapper = new ObjectMapper();
173125
var data = objectMapper.createObjectNode()
@@ -192,9 +144,9 @@ private Pair<OAuth2AccessToken, OAuth2RefreshToken> refreshEclipseToken(AuthToke
192144
var newRefreshToken = new OAuth2RefreshToken(newRefreshTokenValue, issuedAt);
193145
return Pair.of(newToken, newRefreshToken);
194146
} catch (RestClientException exc) {
195-
logger.error("Post request failed with URL: " + tokenUri, exc);
147+
logger.error("Post request failed with URL: {}", tokenUri, exc);
196148
} catch (JsonProcessingException exc) {
197-
logger.error("Invalid JSON data received from URL: " + tokenUri, exc);
149+
logger.error("Invalid JSON data received from URL: {}", tokenUri, exc);
198150
}
199151
return null;
200152
}

‎server/src/main/java/org/eclipse/openvsx/entities/UserData.java‎

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,14 @@
99
********************************************************************************/
1010
package org.eclipse.openvsx.entities;
1111

12+
import jakarta.persistence.*;
13+
import org.eclipse.openvsx.json.UserJson;
14+
1215
import java.io.Serial;
1316
import java.io.Serializable;
1417
import java.util.List;
1518
import java.util.Objects;
1619

17-
import jakarta.persistence.*;
18-
19-
import org.eclipse.openvsx.json.UserJson;
20-
2120
@Entity
2221
public class UserData implements Serializable {
2322

@@ -58,10 +57,6 @@ public class UserData implements Serializable {
5857

5958
private String eclipsePersonId;
6059

61-
@Column(length = 4096)
62-
@Convert(converter = AuthTokenConverter.class)
63-
private AuthToken githubToken;
64-
6560
@Column(length = 4096)
6661
@Convert(converter = AuthTokenConverter.class)
6762
private AuthToken eclipseToken;
@@ -160,14 +155,6 @@ public void setEclipsePersonId(String eclipsePersonId) {
160155
this.eclipsePersonId = eclipsePersonId;
161156
}
162157

163-
public AuthToken getGithubToken() {
164-
return githubToken;
165-
}
166-
167-
public void setGithubToken(AuthToken githubToken) {
168-
this.githubToken = githubToken;
169-
}
170-
171158
public AuthToken getEclipseToken() {
172159
return eclipseToken;
173160
}
@@ -193,15 +180,14 @@ public boolean equals(Object o) {
193180
&& Objects.equals(tokens, userData.tokens)
194181
&& Objects.equals(memberships, userData.memberships)
195182
&& Objects.equals(eclipsePersonId, userData.eclipsePersonId)
196-
&& Objects.equals(githubToken, userData.githubToken)
197183
&& Objects.equals(eclipseToken, userData.eclipseToken);
198184
}
199185

200186
@Override
201187
public int hashCode() {
202188
return Objects.hash(
203189
id, role, loginName, fullName, email, avatarUrl, provider, authId, providerUrl, tokens, memberships,
204-
eclipsePersonId, githubToken, eclipseToken
190+
eclipsePersonId, eclipseToken
205191
);
206192
}
207193
}

‎server/src/main/java/org/eclipse/openvsx/security/OAuth2UserServices.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import org.apache.commons.lang3.StringUtils;
1414
import org.eclipse.openvsx.UserService;
1515
import org.eclipse.openvsx.eclipse.EclipseService;
16+
import org.eclipse.openvsx.eclipse.TokenService;
1617
import org.eclipse.openvsx.entities.UserData;
1718
import org.eclipse.openvsx.repositories.RepositoryService;
1819
import org.eclipse.openvsx.util.ErrorResultException;
@@ -89,12 +90,11 @@ public void authenticationSucceeded(AuthenticationSuccessEvent event) {
8990
// `ExtendedOAuth2UserServices.loadUser` was processed.
9091
if (event.getSource() instanceof OAuth2LoginAuthenticationToken) {
9192
var auth = (OAuth2LoginAuthenticationToken) event.getSource();
92-
var idPrincipal = (IdPrincipal) auth.getPrincipal();
93-
var accessToken = auth.getAccessToken();
94-
var refreshToken = auth.getRefreshToken();
9593
var registrationId = auth.getClientRegistration().getRegistrationId();
96-
97-
tokens.updateTokens(idPrincipal.getId(), registrationId, accessToken, refreshToken);
94+
if(registrationId.equals("eclipse")) {
95+
var idPrincipal = (IdPrincipal) auth.getPrincipal();
96+
tokens.updateEclipseToken(idPrincipal.getId(), auth.getAccessToken(), auth.getRefreshToken());
97+
}
9898
}
9999
}
100100

‎server/src/main/resources/db/migration/V1_51__UserData_Drop_GithubToken.sql‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE user_data DROP COLUMN github_token;

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import org.eclipse.openvsx.cache.ExtensionJsonCacheKeyGenerator;
2020
import org.eclipse.openvsx.cache.LatestExtensionVersionCacheKeyGenerator;
2121
import org.eclipse.openvsx.eclipse.EclipseService;
22+
import org.eclipse.openvsx.eclipse.TokenService;
2223
import org.eclipse.openvsx.entities.*;
2324
import org.eclipse.openvsx.extension_control.ExtensionControlService;
2425
import org.eclipse.openvsx.json.*;
@@ -31,7 +32,6 @@
3132
import org.eclipse.openvsx.search.SearchUtilService;
3233
import org.eclipse.openvsx.security.OAuth2UserServices;
3334
import org.eclipse.openvsx.security.SecurityConfig;
34-
import org.eclipse.openvsx.security.TokenService;
3535
import org.eclipse.openvsx.storage.*;
3636
import org.eclipse.openvsx.util.TargetPlatform;
3737
import org.eclipse.openvsx.util.VersionAlias;
@@ -76,7 +76,8 @@
7676
import java.util.zip.ZipOutputStream;
7777

7878
import static org.eclipse.openvsx.entities.FileResource.*;
79-
import static org.mockito.ArgumentMatchers.*;
79+
import static org.mockito.ArgumentMatchers.any;
80+
import static org.mockito.ArgumentMatchers.anyCollection;
8081
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf;
8182
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.user;
8283
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;

0 commit comments

Comments
 (0)