Skip to content

Commit

Permalink
feat: make OAuth2 configuration optional
Browse files Browse the repository at this point in the history
Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>
  • Loading branch information
svor committed Dec 3, 2024
1 parent 3f08d6d commit 821acc9
Show file tree
Hide file tree
Showing 13 changed files with 139 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,6 @@ public interface IExtensionRegistry {
String getPublicKey(String publicId);

RegistryVersionJson getRegistryVersion();

boolean isOAuth2Enabled();
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand All @@ -86,6 +90,7 @@ public LocalRegistryService(
this.eclipse = eclipse;
this.cache = cache;
this.integrityService = integrityService;
this.clientRegistrationRepository = clientRegistrationRepository;
}

@Value("${ovsx.webui.url:}")
Expand Down Expand Up @@ -1140,4 +1145,9 @@ public RegistryVersionJson getRegistryVersion() {
registryVersion.setVersion(this.registryVersion);
return registryVersion;
}

@Override
public boolean isOAuth2Enabled() {
return this.clientRegistrationRepository == null ? false : true;
}
}
34 changes: 34 additions & 0 deletions server/src/main/java/org/eclipse/openvsx/RegistryAPI.java
Original file line number Diff line number Diff line change
Expand Up @@ -1773,4 +1773,38 @@ public ResponseEntity<RegistryVersionJson> 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<Boolean> 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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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/**"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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);
}
Expand Down
6 changes: 4 additions & 2 deletions server/src/test/java/org/eclipse/openvsx/RegistryAPITest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2425,7 +2425,8 @@ LocalRegistryService localRegistryService(
StorageUtilService storageUtil,
EclipseService eclipse,
CacheService cache,
ExtensionVersionIntegrityService integrityService
ExtensionVersionIntegrityService integrityService,
ClientRegistrationRepository clientRegistrationRepository
) {
return new LocalRegistryService(
entityManager,
Expand All @@ -2438,7 +2439,8 @@ LocalRegistryService localRegistryService(
storageUtil,
eclipse,
cache,
integrityService
integrityService,
clientRegistrationRepository
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1250,7 +1250,8 @@ LocalRegistryService localRegistryService(
StorageUtilService storageUtil,
EclipseService eclipse,
CacheService cache,
ExtensionVersionIntegrityService integrityService
ExtensionVersionIntegrityService integrityService,
ClientRegistrationRepository clientRegistrationRepository
) {
return new LocalRegistryService(
entityManager,
Expand All @@ -1263,7 +1264,8 @@ LocalRegistryService localRegistryService(
storageUtil,
eclipse,
cache,
integrityService
integrityService,
clientRegistrationRepository
);
}

Expand Down
1 change: 1 addition & 0 deletions webui/src/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export interface MainContext {
handleError: (err: Error | Partial<ErrorResponse>) => void;
user?: UserData;
updateUser: () => void;
isOAuth2Enabled: boolean;
}

// We don't include `undefined` as context value to avoid checking the value in all components
Expand Down
21 changes: 12 additions & 9 deletions webui/src/default/menu-content.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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 --------------------//

Expand All @@ -44,7 +45,7 @@ const MobileMenuItemText: FunctionComponent<PropsWithChildren> = ({ children })
};

export const MobileMenuContent: FunctionComponent = () => {

const { isOAuth2Enabled } = useContext(MainContext);
const location = useLocation();

return <>
Expand Down Expand Up @@ -81,17 +82,16 @@ export const MobileMenuContent: FunctionComponent = () => {
</RouteLink>
</MobileMenuItem>
{
!location.pathname.startsWith(UserSettingsRoutes.ROOT)
? <MobileMenuItem>
!location.pathname.startsWith(UserSettingsRoutes.ROOT) && isOAuth2Enabled && (
<MobileMenuItem>
<RouteLink to='/user-settings/extensions'>
<MobileMenuItemText>
<PublishIcon sx={itemIcon} />
Publish Extension
</MobileMenuItemText>
</RouteLink>
</MobileMenuItem>
: null
}
)}
</>;
};

Expand All @@ -115,6 +115,7 @@ const MenuLink = styled(Link)(headerItem);
const MenuRouteLink = styled(RouteLink)(headerItem);

export const DefaultMenuContent: FunctionComponent = () => {
const { isOAuth2Enabled } = useContext(MainContext);
return <>
<MenuLink href='https://github.com/eclipse/openvsx/wiki'>
Documentation
Expand All @@ -125,8 +126,10 @@ export const DefaultMenuContent: FunctionComponent = () => {
<MenuRouteLink to='/about'>
About
</MenuRouteLink>
<Button variant='contained' color='secondary' href='/user-settings/extensions' sx={{ mx: 2.5 }}>
Publish
</Button>
{isOAuth2Enabled && (
<Button variant='contained' color='secondary' href='/user-settings/extensions' sx={{ mx: 2.5 }}>
Publish
</Button>
)}
</>;
};
5 changes: 5 additions & 0 deletions webui/src/extension-registry-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,11 @@ export class ExtensionRegistryService {
const endpoint = createAbsoluteURL([this.serverUrl, 'api', 'version']);
return sendRequest({ abortController, endpoint });
}

async isOAuth2Enabled(abortController: AbortController): Promise<Readonly<boolean>> {
const endpoint = createAbsoluteURL([this.serverUrl, 'api', 'oauth2', 'enabled']);
return sendRequest({ abortController, endpoint });
}
}

export interface AdminService {
Expand Down
14 changes: 14 additions & 0 deletions webui/src/main.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { OtherPages } from './other-pages';
export const Main: FunctionComponent<MainProps> = props => {
const [user, setUser] = useState<UserData>();
const [userLoading, setUserLoading] = useState<boolean>(true);
const [isOAuth2Enabled, setIsOAuth2Enabled] = useState<boolean>(false);
const [error, setError] = useState<{message: string, code?: number | string}>();
const [isErrorDialogOpen, setIsErrorDialogOpen] = useState<boolean>(false);
const abortController = useRef<AbortController>(new AbortController());
Expand All @@ -40,6 +41,9 @@ export const Main: FunctionComponent<MainProps> = props => {
// Get data of the currently logged in user
updateUser();

// Fetch OAuth2 status
fetchOAuth2Status();

return () => abortController.current.abort();
}, []);

Expand All @@ -60,6 +64,15 @@ export const Main: FunctionComponent<MainProps> = 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<ErrorResponse> | ReportedError) => {
if (err instanceof DOMException && err.message.trim() === 'The operation was aborted.') {
// ignore error caused by AbortController.abort()
Expand Down Expand Up @@ -101,6 +114,7 @@ export const Main: FunctionComponent<MainProps> = props => {
pageSettings: props.pageSettings,
user,
updateUser,
isOAuth2Enabled,
handleError: onError
};
return <>
Expand Down
20 changes: 11 additions & 9 deletions webui/src/other-pages.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const Footer = styled('footer')(({ theme }: { theme: Theme }) => ({
}));

export const OtherPages: FunctionComponent<OtherPagesProps> = (props) => {
const { service, pageSettings } = useContext(MainContext);
const { service, pageSettings, isOAuth2Enabled } = useContext(MainContext);
const {
additionalRoutes: AdditionalRoutes,
banner: BannerComponent,
Expand Down Expand Up @@ -90,15 +90,17 @@ export const OtherPages: FunctionComponent<OtherPagesProps> = (props) => {
<ToolbarItem>
<HeaderMenu />
{
props.user ?
props.user ? (
<UserAvatar />
:
<IconButton
href={service.getLoginUrl()}
title='Log In'
aria-label='Log In' >
<AccountBoxIcon />
</IconButton>
) : (
isOAuth2Enabled && ( // Show "Log In" button only if OAuth2 is enabled
<IconButton
href={service.getLoginUrl()}
title='Log In'
aria-label='Log In' >
<AccountBoxIcon />
</IconButton>)
)
}
</ToolbarItem>
</Toolbar>
Expand Down

0 comments on commit 821acc9

Please sign in to comment.