From 821acc9b2793ca7427f4d284ea79deaabb4399e1 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 | 21 +++++++----- webui/src/extension-registry-service.ts | 5 +++ webui/src/main.tsx | 14 ++++++++ webui/src/other-pages.tsx | 20 ++++++----- 13 files changed, 139 insertions(+), 24 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 37fa6f289..de350bd41 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; @@ -61,6 +63,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, @@ -73,7 +76,8 @@ public LocalRegistryService( StorageUtilService storageUtil, EclipseService eclipse, CacheService cache, - ExtensionVersionIntegrityService integrityService + ExtensionVersionIntegrityService integrityService, + @Autowired(required = false) ClientRegistrationRepository clientRegistrationRepository ) { this.entityManager = entityManager; this.repositories = repositories; @@ -86,6 +90,7 @@ public LocalRegistryService( this.eclipse = eclipse; this.cache = cache; this.integrityService = integrityService; + this.clientRegistrationRepository = clientRegistrationRepository; } @Value("${ovsx.webui.url:}") @@ -1140,4 +1145,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 c24671ef5..e0a20ffe0 100644 --- a/server/src/main/java/org/eclipse/openvsx/RegistryAPI.java +++ b/server/src/main/java/org/eclipse/openvsx/RegistryAPI.java @@ -1773,4 +1773,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 3fd4878ab..19e2051b6 100644 --- a/server/src/main/java/org/eclipse/openvsx/UpstreamRegistryService.java +++ b/server/src/main/java/org/eclipse/openvsx/UpstreamRegistryService.java @@ -418,6 +418,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 aa32ff073..3c11f0147 100644 --- a/server/src/test/java/org/eclipse/openvsx/RegistryAPITest.java +++ b/server/src/test/java/org/eclipse/openvsx/RegistryAPITest.java @@ -2425,7 +2425,8 @@ LocalRegistryService localRegistryService( StorageUtilService storageUtil, EclipseService eclipse, CacheService cache, - ExtensionVersionIntegrityService integrityService + ExtensionVersionIntegrityService integrityService, + ClientRegistrationRepository clientRegistrationRepository ) { return new LocalRegistryService( entityManager, @@ -2438,7 +2439,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 7a36e370a..ecd7bbb80 100644 --- a/server/src/test/java/org/eclipse/openvsx/admin/AdminAPITest.java +++ b/server/src/test/java/org/eclipse/openvsx/admin/AdminAPITest.java @@ -1250,7 +1250,8 @@ LocalRegistryService localRegistryService( StorageUtilService storageUtil, EclipseService eclipse, CacheService cache, - ExtensionVersionIntegrityService integrityService + ExtensionVersionIntegrityService integrityService, + ClientRegistrationRepository clientRegistrationRepository ) { return new LocalRegistryService( entityManager, @@ -1263,7 +1264,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 0345216a8..e47f97881 100644 --- a/webui/src/default/menu-content.tsx +++ b/webui/src/default/menu-content.tsx @@ -8,7 +8,7 @@ * SPDX-License-Identifier: EPL-2.0 ********************************************************************************/ -import React, { FunctionComponent, PropsWithChildren } from 'react'; +import React, { FunctionComponent, PropsWithChildren, useContext } from 'react'; import { Typography, MenuItem, Link, Button } from '@mui/material'; import { useLocation } from 'react-router-dom'; import { Link as RouteLink } from 'react-router-dom'; @@ -19,6 +19,7 @@ import InfoIcon from '@mui/icons-material/Info'; import PublishIcon from '@mui/icons-material/Publish'; import { UserSettingsRoutes } from '../pages/user/user-settings'; import { styled, Theme } from '@mui/material/styles'; +import { MainContext } from '../context'; //-------------------- Mobile View --------------------// @@ -44,7 +45,7 @@ const MobileMenuItemText: FunctionComponent = ({ children }) }; export const MobileMenuContent: FunctionComponent = () => { - + const { isOAuth2Enabled } = useContext(MainContext); const location = useLocation(); return <> @@ -81,8 +82,8 @@ export const MobileMenuContent: FunctionComponent = () => { { - !location.pathname.startsWith(UserSettingsRoutes.ROOT) - ? + !location.pathname.startsWith(UserSettingsRoutes.ROOT) && isOAuth2Enabled && ( + @@ -90,8 +91,7 @@ export const MobileMenuContent: FunctionComponent = () => { - : null - } + )} ; }; @@ -115,6 +115,7 @@ const MenuLink = styled(Link)(headerItem); const MenuRouteLink = styled(RouteLink)(headerItem); export const DefaultMenuContent: FunctionComponent = () => { + const { isOAuth2Enabled } = useContext(MainContext); return <> Documentation @@ -125,8 +126,10 @@ export const DefaultMenuContent: FunctionComponent = () => { About - + {isOAuth2Enabled && ( + + )} ; }; diff --git a/webui/src/extension-registry-service.ts b/webui/src/extension-registry-service.ts index ce369ee7e..ecd73aebc 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 <> diff --git a/webui/src/other-pages.tsx b/webui/src/other-pages.tsx index 648a0b80e..890143382 100644 --- a/webui/src/other-pages.tsx +++ b/webui/src/other-pages.tsx @@ -38,7 +38,7 @@ const Footer = styled('footer')(({ theme }: { theme: Theme }) => ({ })); export const OtherPages: FunctionComponent = (props) => { - const { service, pageSettings } = useContext(MainContext); + const { service, pageSettings, isOAuth2Enabled } = useContext(MainContext); const { additionalRoutes: AdditionalRoutes, banner: BannerComponent, @@ -90,15 +90,17 @@ export const OtherPages: FunctionComponent = (props) => { { - props.user ? + props.user ? ( - : - - - + ) : ( + isOAuth2Enabled && ( // Show "Log In" button only if OAuth2 is enabled + + + ) + ) }