Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove github token #1132

Merged
merged 1 commit into from
Feb 27, 2025
Merged

Remove github token #1132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions server/src/main/java/org/eclipse/openvsx/eclipse/EclipseService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import org.eclipse.openvsx.entities.AuthToken;
import org.eclipse.openvsx.entities.UserData;
import org.eclipse.openvsx.json.UserJson;
import org.eclipse.openvsx.security.TokenService;
import org.eclipse.openvsx.util.ErrorResultException;
import org.eclipse.openvsx.util.TimeUtil;
import org.eclipse.openvsx.util.UrlUtil;
Expand Down Expand Up @@ -455,7 +454,7 @@ private void checkApiUrl() {
}

private AuthToken checkEclipseToken(UserData user) {
var eclipseToken = tokens.getActiveToken(user, "eclipse");
var eclipseToken = tokens.getActiveEclipseToken(user);
if (eclipseToken == null || StringUtils.isEmpty(eclipseToken.accessToken())) {
throw new ErrorResultException("Authorization by Eclipse required.", HttpStatus.FORBIDDEN);
}
Expand Down
140 changes: 46 additions & 94 deletions ...clipse/openvsx/security/TokenService.java → ...eclipse/openvsx/eclipse/TokenService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*
* SPDX-License-Identifier: EPL-2.0
********************************************************************************/
package org.eclipse.openvsx.security;
package org.eclipse.openvsx.eclipse;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
Expand All @@ -16,6 +16,7 @@
import org.eclipse.openvsx.entities.UserData;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.data.util.Pair;
import org.springframework.http.HttpEntity;
import org.springframework.http.HttpHeaders;
Expand All @@ -28,10 +29,10 @@
import org.springframework.transaction.support.TransactionTemplate;
import org.springframework.web.client.RestClientException;
import org.springframework.web.client.RestTemplate;
import org.springframework.beans.factory.annotation.Autowired;

import java.time.Instant;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;

@Component
public class TokenService {
Expand All @@ -52,105 +53,51 @@ public TokenService(
this.clientRegistrationRepository = clientRegistrationRepository;
}

private boolean isEnabled() {
return clientRegistrationRepository != null;
}

public AuthToken updateTokens(long userId, String registrationId, OAuth2AccessToken accessToken,
OAuth2RefreshToken refreshToken) {
var userData = isEnabled() ? entityManager.find(UserData.class, userId) : null;
if (userData == null) {
return null;
}

switch (registrationId) {
case "github": {
if (accessToken == null) {
return updateGitHubToken(userData, null);
}

var token = new AuthToken(
accessToken.getTokenValue(),
accessToken.getIssuedAt(),
accessToken.getExpiresAt(),
accessToken.getScopes(),
null,
null
);
return updateGitHubToken(userData, token);
}

case "eclipse": {
if (accessToken == null) {
return updateEclipseToken(userData, null);
}

String refresh = null;
Instant refreshExpiresAt = null;
if (refreshToken != null) {
refresh = refreshToken.getTokenValue();
refreshExpiresAt = refreshToken.getExpiresAt();
}

var token = new AuthToken(
accessToken.getTokenValue(),
accessToken.getIssuedAt(),
accessToken.getExpiresAt(),
accessToken.getScopes(),
refresh,
refreshExpiresAt
);

return updateEclipseToken(userData, token);
}
}
return null;
}

private AuthToken updateGitHubToken(UserData userData, AuthToken token) {
return transactions.execute(status -> {
userData.setGithubToken(token);
entityManager.merge(userData);
return token;
});
}

private AuthToken updateEclipseToken(UserData userData, AuthToken token) {
public AuthToken updateEclipseToken(long userId, OAuth2AccessToken accessToken, OAuth2RefreshToken refreshToken) {
var token = toAuthToken(accessToken, refreshToken);
return transactions.execute(status -> {
var userData = entityManager.find(UserData.class, userId);
userData.setEclipseToken(token);
entityManager.merge(userData);
return token;
});
}

public AuthToken getActiveToken(UserData userData, String registrationId) {
if(!isEnabled()) {
private AuthToken toAuthToken(OAuth2AccessToken accessToken, OAuth2RefreshToken refreshToken) {
if(accessToken == null) {
return null;
}

switch (registrationId) {
case "github": {
return userData.getGithubToken();
}
String refresh = null;
Instant refreshExpiresAt = null;
if (refreshToken != null) {
refresh = refreshToken.getTokenValue();
refreshExpiresAt = refreshToken.getExpiresAt();
}

return new AuthToken(
accessToken.getTokenValue(),
accessToken.getIssuedAt(),
accessToken.getExpiresAt(),
accessToken.getScopes(),
refresh,
refreshExpiresAt
);
}

case "eclipse": {
var token = userData.getEclipseToken();
if (token != null && isExpired(token.expiresAt())) {
OAuth2AccessToken newAccessToken = null;
OAuth2RefreshToken newRefreshToken = null;
var newTokens = refreshEclipseToken(token);
if (newTokens != null) {
newAccessToken = newTokens.getFirst();
newRefreshToken = newTokens.getSecond();
}

return updateTokens(userData.getId(), "eclipse", newAccessToken, newRefreshToken);
}
return token;
public AuthToken getActiveEclipseToken(UserData userData) {
var token = userData.getEclipseToken();
if (token != null && isExpired(token.expiresAt())) {
OAuth2AccessToken newAccessToken = null;
OAuth2RefreshToken newRefreshToken = null;
var newTokens = refreshEclipseToken(token);
if (newTokens != null) {
newAccessToken = newTokens.getFirst();
newRefreshToken = newTokens.getSecond();
}
}

return null;
return updateEclipseToken(userData.getId(), newAccessToken, newRefreshToken);
}
return token;
}

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

var reg = clientRegistrationRepository.findByRegistrationId("eclipse");
var reg = Optional.ofNullable(clientRegistrationRepository).map(repo -> repo.findByRegistrationId("eclipse")).orElse(null);
if(reg == null) {
logger.error("Eclipse client not registered");
return null;
}

var tokenUri = reg.getProviderDetails().getTokenUri();

var headers = new HttpHeaders();
headers.setContentType(MediaType.APPLICATION_JSON);
headers.setAccept(Arrays.asList(MediaType.APPLICATION_JSON));
headers.setAccept(List.of(MediaType.APPLICATION_JSON));

var objectMapper = new ObjectMapper();
var data = objectMapper.createObjectNode()
Expand All @@ -192,9 +144,9 @@ private Pair<OAuth2AccessToken, OAuth2RefreshToken> refreshEclipseToken(AuthToke
var newRefreshToken = new OAuth2RefreshToken(newRefreshTokenValue, issuedAt);
return Pair.of(newToken, newRefreshToken);
} catch (RestClientException exc) {
logger.error("Post request failed with URL: " + tokenUri, exc);
logger.error("Post request failed with URL: {}", tokenUri, exc);
} catch (JsonProcessingException exc) {
logger.error("Invalid JSON data received from URL: " + tokenUri, exc);
logger.error("Invalid JSON data received from URL: {}", tokenUri, exc);
}
return null;
}
Expand Down
22 changes: 4 additions & 18 deletions server/src/main/java/org/eclipse/openvsx/entities/UserData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,14 @@
********************************************************************************/
package org.eclipse.openvsx.entities;

import jakarta.persistence.*;
import org.eclipse.openvsx.json.UserJson;

import java.io.Serial;
import java.io.Serializable;
import java.util.List;
import java.util.Objects;

import jakarta.persistence.*;

import org.eclipse.openvsx.json.UserJson;

@Entity
public class UserData implements Serializable {

Expand Down Expand Up @@ -58,10 +57,6 @@ public class UserData implements Serializable {

private String eclipsePersonId;

@Column(length = 4096)
@Convert(converter = AuthTokenConverter.class)
private AuthToken githubToken;

@Column(length = 4096)
@Convert(converter = AuthTokenConverter.class)
private AuthToken eclipseToken;
Expand Down Expand Up @@ -160,14 +155,6 @@ public void setEclipsePersonId(String eclipsePersonId) {
this.eclipsePersonId = eclipsePersonId;
}

public AuthToken getGithubToken() {
return githubToken;
}

public void setGithubToken(AuthToken githubToken) {
this.githubToken = githubToken;
}

public AuthToken getEclipseToken() {
return eclipseToken;
}
Expand All @@ -193,15 +180,14 @@ public boolean equals(Object o) {
&& Objects.equals(tokens, userData.tokens)
&& Objects.equals(memberships, userData.memberships)
&& Objects.equals(eclipsePersonId, userData.eclipsePersonId)
&& Objects.equals(githubToken, userData.githubToken)
&& Objects.equals(eclipseToken, userData.eclipseToken);
}

@Override
public int hashCode() {
return Objects.hash(
id, role, loginName, fullName, email, avatarUrl, provider, authId, providerUrl, tokens, memberships,
eclipsePersonId, githubToken, eclipseToken
eclipsePersonId, eclipseToken
);
}
}
10 changes: 5 additions & 5 deletions server/src/main/java/org/eclipse/openvsx/security/OAuth2UserServices.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.apache.commons.lang3.StringUtils;
import org.eclipse.openvsx.UserService;
import org.eclipse.openvsx.eclipse.EclipseService;
import org.eclipse.openvsx.eclipse.TokenService;
import org.eclipse.openvsx.entities.UserData;
import org.eclipse.openvsx.repositories.RepositoryService;
import org.eclipse.openvsx.util.ErrorResultException;
Expand Down Expand Up @@ -89,12 +90,11 @@ public void authenticationSucceeded(AuthenticationSuccessEvent event) {
// `ExtendedOAuth2UserServices.loadUser` was processed.
if (event.getSource() instanceof OAuth2LoginAuthenticationToken) {
var auth = (OAuth2LoginAuthenticationToken) event.getSource();
var idPrincipal = (IdPrincipal) auth.getPrincipal();
var accessToken = auth.getAccessToken();
var refreshToken = auth.getRefreshToken();
var registrationId = auth.getClientRegistration().getRegistrationId();

tokens.updateTokens(idPrincipal.getId(), registrationId, accessToken, refreshToken);
if(registrationId.equals("eclipse")) {
var idPrincipal = (IdPrincipal) auth.getPrincipal();
tokens.updateEclipseToken(idPrincipal.getId(), auth.getAccessToken(), auth.getRefreshToken());
}
}
}

Expand Down
1 change: 1 addition & 0 deletions server/src/main/resources/db/migration/V1_51__UserData_Drop_GithubToken.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE user_data DROP COLUMN github_token;
5 changes: 3 additions & 2 deletions server/src/test/java/org/eclipse/openvsx/RegistryAPITest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.eclipse.openvsx.cache.ExtensionJsonCacheKeyGenerator;
import org.eclipse.openvsx.cache.LatestExtensionVersionCacheKeyGenerator;
import org.eclipse.openvsx.eclipse.EclipseService;
import org.eclipse.openvsx.eclipse.TokenService;
import org.eclipse.openvsx.entities.*;
import org.eclipse.openvsx.extension_control.ExtensionControlService;
import org.eclipse.openvsx.json.*;
Expand All @@ -31,7 +32,6 @@
import org.eclipse.openvsx.search.SearchUtilService;
import org.eclipse.openvsx.security.OAuth2UserServices;
import org.eclipse.openvsx.security.SecurityConfig;
import org.eclipse.openvsx.security.TokenService;
import org.eclipse.openvsx.storage.*;
import org.eclipse.openvsx.util.TargetPlatform;
import org.eclipse.openvsx.util.VersionAlias;
Expand Down Expand Up @@ -76,7 +76,8 @@
import java.util.zip.ZipOutputStream;

import static org.eclipse.openvsx.entities.FileResource.*;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyCollection;
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf;
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.user;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
Expand Down
Loading