From 08da93d9e35fc23d166dba4eae3f95d5cd3fdd6d Mon Sep 17 00:00:00 2001 From: Valeriy Svydenko Date: Tue, 3 Dec 2024 13:15:53 +0200 Subject: [PATCH] feat: make OAuth2 configuration optional Signed-off-by: Valeriy Svydenko --- .../eclipse/openvsx/IExtensionRegistry.java | 2 + .../eclipse/openvsx/LocalRegistryService.java | 12 ++- .../java/org/eclipse/openvsx/RegistryAPI.java | 34 ++++++++ .../openvsx/UpstreamRegistryService.java | 11 +++ .../openvsx/security/SecurityConfig.java | 24 ++++++ .../openvsx/security/TokenService.java | 7 +- .../org/eclipse/openvsx/RegistryAPITest.java | 6 +- .../eclipse/openvsx/admin/AdminAPITest.java | 6 +- webui/src/context.ts | 1 + webui/src/default/menu-content.tsx | 86 +++++++++++-------- webui/src/extension-registry-service.ts | 5 ++ webui/src/main.tsx | 14 +++ 12 files changed, 165 insertions(+), 43 deletions(-) diff --git a/server/src/main/java/org/eclipse/openvsx/IExtensionRegistry.java b/server/src/main/java/org/eclipse/openvsx/IExtensionRegistry.java index 3f705fff1..de889132e 100644 --- a/server/src/main/java/org/eclipse/openvsx/IExtensionRegistry.java +++ b/server/src/main/java/org/eclipse/openvsx/IExtensionRegistry.java @@ -46,4 +46,6 @@ public interface IExtensionRegistry { String getPublicKey(String publicId); RegistryVersionJson getRegistryVersion(); + + boolean isOAuth2Enabled(); } \ No newline at end of file diff --git a/server/src/main/java/org/eclipse/openvsx/LocalRegistryService.java b/server/src/main/java/org/eclipse/openvsx/LocalRegistryService.java index 806d97fae..5116dbe2c 100644 --- a/server/src/main/java/org/eclipse/openvsx/LocalRegistryService.java +++ b/server/src/main/java/org/eclipse/openvsx/LocalRegistryService.java @@ -26,12 +26,14 @@ import org.eclipse.openvsx.util.*; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.cache.annotation.Cacheable; import org.springframework.data.domain.PageRequest; import org.springframework.data.elasticsearch.core.SearchHits; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; +import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository; import org.springframework.stereotype.Component; import org.springframework.web.server.ResponseStatusException; import org.springframework.web.servlet.mvc.method.annotation.StreamingResponseBody; @@ -63,6 +65,7 @@ public class LocalRegistryService implements IExtensionRegistry { private final EclipseService eclipse; private final CacheService cache; private final ExtensionVersionIntegrityService integrityService; + private final ClientRegistrationRepository clientRegistrationRepository; public LocalRegistryService( EntityManager entityManager, @@ -75,7 +78,8 @@ public LocalRegistryService( StorageUtilService storageUtil, EclipseService eclipse, CacheService cache, - ExtensionVersionIntegrityService integrityService + ExtensionVersionIntegrityService integrityService, + @Autowired(required = false) ClientRegistrationRepository clientRegistrationRepository ) { this.entityManager = entityManager; this.repositories = repositories; @@ -88,6 +92,7 @@ public LocalRegistryService( this.eclipse = eclipse; this.cache = cache; this.integrityService = integrityService; + this.clientRegistrationRepository = clientRegistrationRepository; } @Value("${ovsx.webui.url:}") @@ -1142,4 +1147,9 @@ public RegistryVersionJson getRegistryVersion() { registryVersion.setVersion(this.registryVersion); return registryVersion; } + + @Override + public boolean isOAuth2Enabled() { + return this.clientRegistrationRepository == null ? false : true; + } } diff --git a/server/src/main/java/org/eclipse/openvsx/RegistryAPI.java b/server/src/main/java/org/eclipse/openvsx/RegistryAPI.java index 74b835cad..6486d76d9 100644 --- a/server/src/main/java/org/eclipse/openvsx/RegistryAPI.java +++ b/server/src/main/java/org/eclipse/openvsx/RegistryAPI.java @@ -1404,4 +1404,38 @@ public ResponseEntity getServerVersion() { return exc.toResponseEntity(RegistryVersionJson.class); } } + + @GetMapping(path = "/api/oauth2/enabled", produces = MediaType.APPLICATION_JSON_VALUE) + @CrossOrigin + @Operation(summary = "Check if OAuth2 is enabled") + @ApiResponse( + responseCode = "200", + description = "Returns true if OAuth2 is enabled, false otherwise" + ) + @ApiResponse( + responseCode = "429", + description = "A client has sent too many requests in a given amount of time", + headers = { + @Header( + name = "X-Rate-Limit-Retry-After-Seconds", + description = "Number of seconds to wait before retrying after receiving a 429 response", + schema = @Schema(type = "integer", format = "int32") + ), + @Header( + name = "X-Rate-Limit-Remaining", + description = "Number of remaining requests available", + schema = @Schema(type = "integer", format = "int32") + ) + } + ) + public ResponseEntity isOAuth2Enabled() { + try { + boolean enabled = local.isOAuth2Enabled(); + return ResponseEntity.ok() + .cacheControl(CacheControl.maxAge(5, TimeUnit.MINUTES).cachePublic()) + .body(enabled); + } catch (Exception exc) { + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(false); + } + } } diff --git a/server/src/main/java/org/eclipse/openvsx/UpstreamRegistryService.java b/server/src/main/java/org/eclipse/openvsx/UpstreamRegistryService.java index 0910c093c..40c34cc20 100644 --- a/server/src/main/java/org/eclipse/openvsx/UpstreamRegistryService.java +++ b/server/src/main/java/org/eclipse/openvsx/UpstreamRegistryService.java @@ -428,6 +428,17 @@ public RegistryVersionJson getRegistryVersion() { } } + /** + * For the upstream registry, it is assumed that OAuth2 is always configured and required. + * This method consistently returns {@code true} to reflect that assumption. + * + * @return {@code true}, indicating that OAuth2 is enabled and expected to be configured. + */ + @Override + public boolean isOAuth2Enabled() { + return true; + } + private void handleError(Throwable exc) throws RuntimeException { if (exc instanceof HttpStatusCodeException) { var status = ((HttpStatusCodeException) exc).getStatusCode(); diff --git a/server/src/main/java/org/eclipse/openvsx/security/SecurityConfig.java b/server/src/main/java/org/eclipse/openvsx/security/SecurityConfig.java index 7b9c2fa6c..1c9a3b01f 100644 --- a/server/src/main/java/org/eclipse/openvsx/security/SecurityConfig.java +++ b/server/src/main/java/org/eclipse/openvsx/security/SecurityConfig.java @@ -10,11 +10,13 @@ package org.eclipse.openvsx.security; import org.apache.commons.lang3.StringUtils; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; +import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository; import org.springframework.security.web.SecurityFilterChain; import org.springframework.security.web.authentication.Http403ForbiddenEntryPoint; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; @@ -30,9 +32,31 @@ public class SecurityConfig { @Value("${ovsx.webui.frontendRoutes:/extension/**,/namespace/**,/user-settings/**,/admin-dashboard/**}") String[] frontendRoutes; + private final ClientRegistrationRepository clientRegistrationRepository; + + @Autowired + public SecurityConfig(@Autowired(required = false) ClientRegistrationRepository clientRegistrationRepository) { + this.clientRegistrationRepository = clientRegistrationRepository; + } + @Bean public SecurityFilterChain filterChain(HttpSecurity http, OAuth2UserServices userServices) throws Exception { var redirectUrl = StringUtils.isEmpty(webuiUrl) ? "/" : webuiUrl; + + if (clientRegistrationRepository == null) { + // Minimal security configuration when OAuth2 is not available + return http.authorizeHttpRequests( + registry -> registry + .anyRequest() + .permitAll()) + .cors(configurer -> configurer.configure(http)) + .csrf(configurer -> { + configurer.ignoringRequestMatchers(antMatchers("/api/-/publish", "/api/-/namespace/create", "/api/-/query", "/vscode/**")); + }) + .exceptionHandling(configurer -> configurer.authenticationEntryPoint(new Http403ForbiddenEntryPoint())) + .build(); + } + return http.authorizeHttpRequests( registry -> registry .requestMatchers(antMatchers("/*", "/login/**", "/oauth2/**", "/user", "/user/auth-error", "/logout", "/actuator/health/**", "/actuator/metrics", "/actuator/metrics/**", "/actuator/prometheus", "/v3/api-docs/**", "/swagger-resources/**", "/swagger-ui/**", "/webjars/**")) diff --git a/server/src/main/java/org/eclipse/openvsx/security/TokenService.java b/server/src/main/java/org/eclipse/openvsx/security/TokenService.java index 53546d952..c8cb0be7e 100644 --- a/server/src/main/java/org/eclipse/openvsx/security/TokenService.java +++ b/server/src/main/java/org/eclipse/openvsx/security/TokenService.java @@ -28,6 +28,7 @@ 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; @@ -44,7 +45,7 @@ public class TokenService { public TokenService( TransactionTemplate transactions, EntityManager entityManager, - ClientRegistrationRepository clientRegistrationRepository + @Autowired(required = false) ClientRegistrationRepository clientRegistrationRepository ) { this.transactions = transactions; this.entityManager = entityManager; @@ -60,6 +61,10 @@ public AuthToken updateTokens(long userId, String registrationId, OAuth2AccessTo switch (registrationId) { case "github": { + if (clientRegistrationRepository == null) { + // Handle the case where GitHub OAuth2 is not configured + return updateGitHubToken(userData, null); + } if (accessToken == null) { return updateGitHubToken(userData, null); } diff --git a/server/src/test/java/org/eclipse/openvsx/RegistryAPITest.java b/server/src/test/java/org/eclipse/openvsx/RegistryAPITest.java index d39abcdbd..aa1aa1fd0 100644 --- a/server/src/test/java/org/eclipse/openvsx/RegistryAPITest.java +++ b/server/src/test/java/org/eclipse/openvsx/RegistryAPITest.java @@ -2426,7 +2426,8 @@ LocalRegistryService localRegistryService( EclipseService eclipse, CacheService cache, FileCacheDurationConfig fileCacheDurationConfig, - ExtensionVersionIntegrityService integrityService + ExtensionVersionIntegrityService integrityService, + ClientRegistrationRepository clientRegistrationRepository ) { return new LocalRegistryService( entityManager, @@ -2439,7 +2440,8 @@ LocalRegistryService localRegistryService( storageUtil, eclipse, cache, - integrityService + integrityService, + clientRegistrationRepository ); } 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 b9a65926c..b432f1964 100644 --- a/server/src/test/java/org/eclipse/openvsx/admin/AdminAPITest.java +++ b/server/src/test/java/org/eclipse/openvsx/admin/AdminAPITest.java @@ -1251,7 +1251,8 @@ LocalRegistryService localRegistryService( EclipseService eclipse, CacheService cache, FileCacheDurationConfig fileCacheDurationConfig, - ExtensionVersionIntegrityService integrityService + ExtensionVersionIntegrityService integrityService, + ClientRegistrationRepository clientRegistrationRepository ) { return new LocalRegistryService( entityManager, @@ -1264,7 +1265,8 @@ LocalRegistryService localRegistryService( storageUtil, eclipse, cache, - integrityService + integrityService, + clientRegistrationRepository ); } diff --git a/webui/src/context.ts b/webui/src/context.ts index 10c8c5053..790b41b57 100644 --- a/webui/src/context.ts +++ b/webui/src/context.ts @@ -20,6 +20,7 @@ export interface MainContext { handleError: (err: Error | Partial) => void; user?: UserData; updateUser: () => void; + isOAuth2Enabled: boolean; } // We don't include `undefined` as context value to avoid checking the value in all components diff --git a/webui/src/default/menu-content.tsx b/webui/src/default/menu-content.tsx index 7a38fb07f..d21ffd6db 100644 --- a/webui/src/default/menu-content.tsx +++ b/webui/src/default/menu-content.tsx @@ -116,15 +116,16 @@ export const MobileUserAvatar: FunctionComponent = () => { }; export const MobileMenuContent: FunctionComponent = () => { - + const { isOAuth2Enabled } = useContext(MainContext); const location = useLocation(); const { service, user } = useContext(MainContext); return <> - { - user - ? - : + {isOAuth2Enabled && ( + user ? ( + + ) : ( + @@ -132,10 +133,10 @@ export const MobileMenuContent: FunctionComponent = () => { - } - { - !location.pathname.startsWith(UserSettingsRoutes.ROOT) - ? + ) + )} + {isOAuth2Enabled && !location.pathname.startsWith(UserSettingsRoutes.ROOT) && ( + @@ -143,8 +144,7 @@ export const MobileMenuContent: FunctionComponent = () => { - : null - } + )} @@ -200,30 +200,42 @@ export const MenuLink = styled(Link)(headerItem); export const MenuRouteLink = styled(RouteLink)(headerItem); export const DefaultMenuContent: FunctionComponent = () => { - const { service, user } = useContext(MainContext); - return <> - - Documentation - - - Community - - - About - - - { - user ? - - : - - - - } - ; + const { service, user, isOAuth2Enabled } = useContext(MainContext); + + return ( + <> + + Documentation + + + Community + + + About + + {isOAuth2Enabled && ( + <> + + {user ? ( + + ) : ( + + + + )} + + )} + + ); }; diff --git a/webui/src/extension-registry-service.ts b/webui/src/extension-registry-service.ts index 6bb3144fb..a40fdfabf 100644 --- a/webui/src/extension-registry-service.ts +++ b/webui/src/extension-registry-service.ts @@ -421,6 +421,11 @@ export class ExtensionRegistryService { const endpoint = createAbsoluteURL([this.serverUrl, 'api', 'version']); return sendRequest({ abortController, endpoint }); } + + async isOAuth2Enabled(abortController: AbortController): Promise> { + const endpoint = createAbsoluteURL([this.serverUrl, 'api', 'oauth2', 'enabled']); + return sendRequest({ abortController, endpoint }); + } } export interface AdminService { diff --git a/webui/src/main.tsx b/webui/src/main.tsx index 34419329d..60b5831f4 100644 --- a/webui/src/main.tsx +++ b/webui/src/main.tsx @@ -26,6 +26,7 @@ import { OtherPages } from './other-pages'; export const Main: FunctionComponent = props => { const [user, setUser] = useState(); const [userLoading, setUserLoading] = useState(true); + const [isOAuth2Enabled, setIsOAuth2Enabled] = useState(false); const [error, setError] = useState<{message: string, code?: number | string}>(); const [isErrorDialogOpen, setIsErrorDialogOpen] = useState(false); const abortController = useRef(new AbortController()); @@ -40,6 +41,9 @@ export const Main: FunctionComponent = props => { // Get data of the currently logged in user updateUser(); + // Fetch OAuth2 status + fetchOAuth2Status(); + return () => abortController.current.abort(); }, []); @@ -60,6 +64,15 @@ export const Main: FunctionComponent = props => { setUserLoading(false); }; + const fetchOAuth2Status = async () => { + try { + const isEnabled = await props.service.isOAuth2Enabled(abortController.current); + setIsOAuth2Enabled(isEnabled); + } catch (err) { + console.error('Failed to fetch OAuth2 status:', err); + } + }; + const onError = (err: Error | Partial | ReportedError) => { if (err instanceof DOMException && err.message.trim() === 'The operation was aborted.') { // ignore error caused by AbortController.abort() @@ -101,6 +114,7 @@ export const Main: FunctionComponent = props => { pageSettings: props.pageSettings, user, updateUser, + isOAuth2Enabled, handleError: onError }; return <>