Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,10 @@ describe('useResourceSelection', () => {
const { getByTestId } = render(React.createElement(Probe));

await waitFor(() => expect(getByTestId('loading').textContent).toBe('false'));
expect(fetchMock).toHaveBeenCalledWith('/rest/v1/user/resources', { method: 'GET' });
expect(fetchMock).toHaveBeenCalledWith('/rest/v1/user/resources', {
method: 'GET',
headers: { Accept: 'application/json' },
});
expect(getByTestId('selected').textContent).toBe('ASVS,CWE');
});

Expand Down
5 changes: 4 additions & 1 deletion application/frontend/src/hooks/useResourceSelection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ export const useResourceSelection = (): ResourceSelectionState => {

const load = async () => {
try {
const res = await fetch(`${apiUrl}/user/resources`, { method: 'GET' });
const res = await fetch(`${apiUrl}/user/resources`, {
method: 'GET',
headers: { Accept: 'application/json' },
});
if (res.status === 401) {
return; // anonymous / feature not available — not an error
}
Expand Down
66 changes: 66 additions & 0 deletions application/frontend/src/hooks/useUser.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import { act, render, waitFor } from '@testing-library/react';
import React from 'react';

import { useUser } from './useUser';

jest.mock('./useEnvironment', () => ({
useEnvironment: () => ({ name: 'test', apiUrl: '/rest/v1' }),
}));

// react-testing-library v11 has no renderHook; drive the hook via a probe.
type Captured = ReturnType<typeof useUser>;
let captured: Captured;

function Probe(): React.ReactElement {
captured = useUser();
return React.createElement('span', { 'data-testid': 'loading' }, String(captured.loading));
}

function jsonResponse(body: unknown, status = 200): Response {
return {
status,
ok: status >= 200 && status < 300,
json: () => Promise.resolve(body),
text: () => Promise.resolve(typeof body === 'string' ? body : JSON.stringify(body)),
} as unknown as Response;
}

describe('useUser (auth route migration #963)', () => {
const originalLocation = window.location;

beforeEach(() => {
delete (window as any).location;
(window as any).location = { href: '' };
});

afterEach(() => {
(window as any).location = originalLocation;
jest.resetAllMocks();
});

it('GETs /rest/v1/auth/user with Accept: application/json (so anon gets 401, not a Google redirect)', async () => {
const fetchMock = jest.fn().mockResolvedValueOnce(jsonResponse(null, 401));
(global as any).fetch = fetchMock;

const { getByTestId } = render(React.createElement(Probe));
await waitFor(() => expect(getByTestId('loading').textContent).toBe('false'));

expect(fetchMock).toHaveBeenCalledWith('/rest/v1/auth/user', {
method: 'GET',
headers: { Accept: 'application/json' },
});
});

it('login() navigates to /auth/login and logout() to /auth/logout', async () => {
(global as any).fetch = jest.fn().mockResolvedValueOnce(jsonResponse(null, 401));

const { getByTestId } = render(React.createElement(Probe));
await waitFor(() => expect(getByTestId('loading').textContent).toBe('false'));

act(() => captured.login());
expect((window as any).location.href).toBe('/rest/v1/auth/login');

act(() => captured.logout());
expect((window as any).location.href).toBe('/rest/v1/auth/logout');
});
});
6 changes: 3 additions & 3 deletions application/frontend/src/hooks/useUser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export const useUser = () => {

useEffect(() => {
let active = true;
fetch(`${apiUrl}/user`, { method: 'GET' })
fetch(`${apiUrl}/auth/user`, { method: 'GET', headers: { Accept: 'application/json' } })
.then((res) => {
if (res.status === 200) {
return res.text();
Expand Down Expand Up @@ -51,11 +51,11 @@ export const useUser = () => {
}, [apiUrl]);

const login = () => {
window.location.href = `${apiUrl}/login`;
window.location.href = `${apiUrl}/auth/login`;
};

const logout = () => {
window.location.href = `${apiUrl}/logout`;
window.location.href = `${apiUrl}/auth/logout`;
};

return { user, isLoggedIn: user !== null, loading, login, logout };
Expand Down
6 changes: 3 additions & 3 deletions application/frontend/src/pages/chatbot/chatbot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,12 @@ export const Chatbot = () => {
}, [chatMessages]);

function login() {
fetch(`${apiUrl}/user`, { method: 'GET' })
fetch(`${apiUrl}/auth/user`, { method: 'GET', headers: { Accept: 'application/json' } })
.then((response) => {
if (response.status === 200) {
response.text().then((user) => setUser(user));
} else {
window.location.href = `${apiUrl}/login`;
window.location.href = `${apiUrl}/auth/login`;
}
})
.catch((error) => {
Expand Down Expand Up @@ -156,7 +156,7 @@ export const Chatbot = () => {

fetch(`${apiUrl}/completion`, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
headers: { 'Content-Type': 'application/json', Accept: 'application/json' },
body: JSON.stringify({ prompt: currentTerm }),
})
.then(async (response) => {
Expand Down
22 changes: 22 additions & 0 deletions application/tests/admin_imports_api_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,28 @@ def test_admin_imports_disabled_flag_returns_404(self) -> None:
os.environ, {"CRE_ALLOW_IMPORT": "1", "INSECURE_REQUESTS": "1"}, clear=True
)
def test_admin_imports_requires_login(self) -> None:
with self.app.test_client() as c:
# API client (Accept: application/json) gets a 401; browsers (Accept:
# text/html) get a 302 to the login flow (login_required content
# negotiation, #963 — default is now 401).
r = c.get("/admin/imports/runs", headers={"Accept": "application/json"})
self.assertEqual(r.status_code, 401)

@patch.dict(
os.environ, {"CRE_ALLOW_IMPORT": "1", "INSECURE_REQUESTS": "1"}, clear=True
)
def test_admin_imports_star_accept_returns_401(self) -> None:
# /admin/* tooling with curl's default Accept "*/*" must get a clean 401,
# not a 302 into login HTML (the case Spyros called out).
with self.app.test_client() as c:
r = c.get("/admin/imports/runs", headers={"Accept": "*/*"})
self.assertEqual(r.status_code, 401)

@patch.dict(
os.environ, {"CRE_ALLOW_IMPORT": "1", "INSECURE_REQUESTS": "1"}, clear=True
)
def test_admin_imports_no_accept_header_returns_401(self) -> None:
# /admin/* tooling with no Accept header at all -> 401, not 302.
with self.app.test_client() as c:
r = c.get("/admin/imports/runs")
self.assertEqual(r.status_code, 401)
Expand Down
Loading
Loading