Skip to content

Commit

Permalink
Handle Ipsilon error on login
Browse files Browse the repository at this point in the history
Catch the errors in the API and report them in the frontend.

Fixes: #880

Signed-off-by: Aurélien Bompard <[email protected]>
  • Loading branch information
abompard committed Apr 27, 2023
1 parent c3112b2 commit 86d7721
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 20 deletions.
8 changes: 7 additions & 1 deletion fmn/api/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

from fastapi import Depends, HTTPException, status
from fastapi.security import HTTPAuthorizationCredentials, HTTPBearer
from httpx import AsyncClient
from httpx import AsyncClient, HTTPError
from pydantic import BaseModel

from ..core.config import get_settings
Expand Down Expand Up @@ -124,6 +124,12 @@ async def __call__(
if self.optional:
return None
raise HTTPException(status.HTTP_401_UNAUTHORIZED, detail="Token expired") from exc
except HTTPError as exc:
if self.optional:
return None
raise HTTPException(
status.HTTP_401_UNAUTHORIZED, detail=f"Could not get user information: {exc}"
) from exc
if identity is None and not self.optional:
raise HTTPException(status.HTTP_401_UNAUTHORIZED, detail="Not authenticated")
return identity
Expand Down
62 changes: 62 additions & 0 deletions frontend/src/auth/LoginFedora.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
waitFor,
type RenderOptions,
} from "@testing-library/vue";
import { AxiosError, AxiosHeaders, type AxiosResponse } from "axios";
import { createPinia, setActivePinia } from "pinia";
import {
afterEach,
Expand Down Expand Up @@ -150,6 +151,67 @@ describe("LoginFedora", () => {
});
});

it("handles API errors when getting user info", async () => {
tokenResponse = new TokenResponse({
access_token: "dummy-access-token",
});
auth.makeUserInfoRequest.mockResolvedValue({
sub: "dummy-sub",
nickname: "dummy-username",
});

const mockedErrorResponse: AxiosResponse = {
status: 500,
statusText: "Server Error",
headers: AxiosHeaders.from(""),
config: { headers: AxiosHeaders.from("") },
data: { detail: "dummy API error" },
};
vi.mocked(apiClient.get).mockRejectedValue(
new AxiosError<{ detail: string }>(
"dummy error",
"500",
undefined,
undefined,
mockedErrorResponse
)
);

render(LoginFedora, renderOptions);

const store = useUserStore();

await waitFor(() => {
// The call to the API is async
expect(vi.mocked(apiClient.get)).toHaveBeenCalled();
});

// We must be logged out
expect(store.$state).toStrictEqual({
accessToken: null,
refreshToken: null,
idToken: null,
tokenExpiresAt: null,
scopes: [],
username: null,
fullName: null,
email: null,
isAdmin: false,
});
// We must have been redirected
await waitFor(() => {
expect(router.currentRoute.value.path).toBe("/");
});
// There should be a "login failed" toast.
const toastStore = useToastStore();
expect(toastStore.toasts).toHaveLength(1);
expect(toastStore.toasts[0].title).toBe("Login failed!");
expect(toastStore.toasts[0].content).toBe(
"Could not retrieve user information from the API: dummy API error."
);
expect(toastStore.toasts[0].color).toBe("danger");
});

it("redirects to the right place on login", async () => {
tokenResponse = new TokenResponse({
access_token: "dummy-access-token",
Expand Down
41 changes: 23 additions & 18 deletions frontend/src/auth/LoginFedora.vue
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ SPDX-License-Identifier: MIT

<script setup lang="ts">
import { getApiClient } from "@/api";
import type { User } from "@/api/types";
import type { APIError, User } from "@/api/types";
import { useToastStore } from "@/stores/toast";
import { onMounted, ref, watch } from "vue";
import { useRouter } from "vue-router";
Expand Down Expand Up @@ -52,24 +52,29 @@ onMounted(async () => {
try {
await auth.handleAuthorizationRedirect((result) => {
userStore.importTokenResponse(result);
auth
.makeUserInfoRequest(result.accessToken)
.then((userinfo) => {
userStore.importUserInfoResponse(userinfo);
})
.then(() => {
console.log(
"Querying the API to create the user and get their permissions"
);
return getApiClient();
})
.then((apiClient) => {
const url = "/api/v1/users/me";
return apiClient.get<User>(url);
})
.then((response) => {
auth.makeUserInfoRequest(result.accessToken).then(async (userinfo) => {
const apiClient = await getApiClient();
const url = "/api/v1/users/me";
try {
const response = await apiClient.get<User>(url);
userStore.setAdmin(response.data.is_admin || false);
});
// Only import the userinfo response if the API answered.
userStore.importUserInfoResponse(userinfo);
} catch (e) {
console.error(e);
const error = e as APIError;
toastStore.addToast({
color: "danger",
title: "Login failed!",
content: `Could not retrieve user information from the API: ${
error.response?.data?.detail || error.message
}.`,
});
userStore.logout();
const redirectTo = getRedirect();
router.push(redirectTo);
}
});
return result;
});
} catch (err) {
Expand Down
16 changes: 15 additions & 1 deletion tests/api/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import pytest
from fastapi import HTTPException, status
from httpx import AsyncClient
from httpx import AsyncClient, HTTPError

from fmn.api.auth import Identity, IdentityFactory, TokenExpired

Expand Down Expand Up @@ -149,3 +149,17 @@ async def test___call__anon_mandatory(self):
with pytest.raises(HTTPException) as excinfo:
await factory(None)
assert excinfo.value.status_code == status.HTTP_401_UNAUTHORIZED

async def test___call__request_failure(self):
factory = IdentityFactory(optional=False)
factory.process_oidc_auth = mock.Mock(side_effect=HTTPError("dummy error"))
with pytest.raises(HTTPException) as excinfo:
await factory(None)
assert excinfo.value.status_code == status.HTTP_401_UNAUTHORIZED
assert excinfo.value.detail == "Could not get user information: dummy error"

async def test___call__request_failure_optional(self):
factory = IdentityFactory(optional=True)
factory.process_oidc_auth = mock.Mock(side_effect=HTTPError("dummy error"))
result = await factory(None)
assert result is None

0 comments on commit 86d7721

Please sign in to comment.