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.

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

[frontend/backend] correct logout redirection #1790

Merged
merged 4 commits into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 14 additions & 2 deletions openbas-api/src/main/java/io/openbas/rest/helper/RestBehavior.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.core.AuthenticationException;
import org.springframework.validation.FieldError;
import org.springframework.web.bind.MethodArgumentNotValidException;
import org.springframework.web.bind.annotation.ExceptionHandler;
Expand Down Expand Up @@ -92,10 +93,10 @@
}

@ResponseStatus(HttpStatus.UNAUTHORIZED)
@ExceptionHandler(AccessDeniedException.class)
@ExceptionHandler(AuthenticationException.class)
public ValidationErrorBag handleValidationExceptions() {
ValidationErrorBag bag =
new ValidationErrorBag(HttpStatus.UNAUTHORIZED.value(), "ACCESS_DENIED");
new ValidationErrorBag(HttpStatus.UNAUTHORIZED.value(), "AUTHENTIFICATION_FAILED");
ValidationError errors = new ValidationError();
Map<String, ValidationContent> errorsBag = new HashMap<>();
errorsBag.put("username", new ValidationContent("Invalid user or password"));
Expand All @@ -104,6 +105,17 @@
return bag;
}

@ResponseStatus(HttpStatus.NOT_FOUND)
guillaumejparis marked this conversation as resolved.
Show resolved Hide resolved
@ExceptionHandler(AccessDeniedException.class)
public ValidationErrorBag handleAccessDeniedExceptions() {
// When the user does not have the appropriate access rights, return 404 Not Found.
// This response indicates that the resource does not exist, preventing any information
// disclosure
// about the resource and reducing the risk of brute force attacks by not confirming its
// existence
return new ValidationErrorBag(HttpStatus.NOT_FOUND.value(), "NOT_FOUND");

Check warning on line 116 in openbas-api/src/main/java/io/openbas/rest/helper/RestBehavior.java

View check run for this annotation

Codecov / codecov/patch

openbas-api/src/main/java/io/openbas/rest/helper/RestBehavior.java#L116

Added line #L116 was not covered by tests
}

@ResponseStatus(HttpStatus.CONFLICT)
@ExceptionHandler(DataIntegrityViolationException.class)
public ViolationErrorBag handleIntegrityException(DataIntegrityViolationException e) {
Expand Down
3 changes: 2 additions & 1 deletion openbas-api/src/main/java/io/openbas/rest/user/UserApi.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.springframework.data.jpa.domain.Specification;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.access.annotation.Secured;
import org.springframework.security.authentication.BadCredentialsException;
import org.springframework.web.bind.annotation.*;

@RestController
Expand Down Expand Up @@ -82,7 +83,7 @@ public User login(@Valid @RequestBody LoginUserInput input) {
return user;
}
}
throw new AccessDeniedException("Invalid credentials");
throw new BadCredentialsException("Invalid credential.");
}

@PostMapping("/api/reset")
Expand Down
20 changes: 14 additions & 6 deletions openbas-front/src/admin/Index.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Box } from '@mui/material';
import { makeStyles, useTheme } from '@mui/styles';
import { lazy, Suspense } from 'react';
import { Route, Routes, useNavigate } from 'react-router-dom';
import { lazy, Suspense, useEffect } from 'react';
import { Navigate, Route, Routes, useNavigate } from 'react-router-dom';

import type { LoggedHelper } from '../actions/helper';
import { fetchTags } from '../actions/Tag';
Expand Down Expand Up @@ -47,9 +47,13 @@ const Index = () => {
const { logged, settings } = useHelper((helper: LoggedHelper) => {
return { logged: helper.logged(), settings: helper.getPlatformSettings() };
});
if (logged.isOnlyPlayer) {
navigate('/private');
}

useEffect(() => {
if (logged.isOnlyPlayer) {
navigate('/');
}
}, [logged]);

const boxSx = {
flexGrow: 1,
padding: 3,
Expand Down Expand Up @@ -99,7 +103,11 @@ const Index = () => {
<Route path="mitigations" element={errorWrapper(Mitigations)()} />
<Route path="integrations/*" element={errorWrapper(IndexIntegrations)()} />
<Route path="agents/*" element={errorWrapper(IndexAgents)()} />
<Route path="settings/*" element={errorWrapper(IndexSettings)()} />
<Route
path="settings/*"
element={logged.admin ? errorWrapper(IndexSettings)()
: <Navigate to="/" replace={true} />}
/>
{/* Not found */}
<Route path="*" element={<NotFound />} />
</Routes>
Expand Down
1 change: 1 addition & 0 deletions openbas-front/src/admin/components/nav/TopBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ const TopBar: React.FC = () => {
});
const handleLogout = async () => {
await dispatch(logout());
navigate('/');
handleCloseMenu();
};

Expand Down
4 changes: 3 additions & 1 deletion openbas-front/src/private/components/nav/TopBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { AppBar, IconButton, Menu, MenuItem, MenuProps, Toolbar } from '@mui/mat
import { makeStyles, useTheme } from '@mui/styles';
import { useState } from 'react';
import * as React from 'react';
import { Link } from 'react-router-dom';
import { Link, useNavigate } from 'react-router-dom';

import { logout } from '../../../actions/Application';
import { useFormatter } from '../../../components/i18n';
Expand Down Expand Up @@ -38,6 +38,7 @@ const TopBar: React.FC = () => {
const theme = useTheme<Theme>();
const classes = useStyles();
const { t } = useFormatter();
const navigate = useNavigate();
const [open, setOpen] = useState(false);
const [anchorEl, setAnchorEl] = useState<MenuProps['anchorEl']>(null);
const dispatch = useAppDispatch();
Expand All @@ -51,6 +52,7 @@ const TopBar: React.FC = () => {
};
const handleLogout = async () => {
await dispatch(logout());
navigate('/');
setOpen(false);
};
return (
Expand Down
5 changes: 1 addition & 4 deletions openbas-front/src/root.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { CssBaseline } from '@mui/material';
import { StyledEngineProvider } from '@mui/material/styles';
import * as R from 'ramda';
import { lazy, Suspense, useEffect } from 'react';
import { Navigate, Route, Routes } from 'react-router-dom';

Expand Down Expand Up @@ -35,9 +34,7 @@ const Root = () => {
dispatch(fetchMe());
dispatch(fetchPlatformParameters());
}, []);
if (R.isEmpty(logged)) {
return <div />;
}

if (!logged || !me || !settings) {
return (
<Suspense fallback={<Loader />}>
Expand Down
4 changes: 2 additions & 2 deletions openbas-front/src/utils/Action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ const notifyError = (error: AxiosError) => {
locale: LANG,
messages: i18n.messages[LANG as keyof typeof i18n.messages],
}, cache);
if (error.status === 401) {
// Do not notify the user, as a 401 error will already trigger a disconnection
if (error.status === 401 || error.status === 404) {
// Do not notify the user, as a 401 error will already trigger a disconnection, as 404 already handle inside the app
} else if (error.status === 409) {
MESSAGING$.notifyError(intl.formatMessage({ id: 'The element already exists' }));
} else if (error.status === 500) {
Expand Down