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

Client Error: TypeError: resp is undefined on transaction submission #31

Merged
merged 2 commits into from
Nov 2, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
15 changes: 8 additions & 7 deletions src/api/auth/AuthProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,20 @@ const AuthProvider: FC<{ children: ReactNode }> = ({ children }) => {
setCookies(response.data);
}

async function logout() {
console.log('Logout called')
clearCookies();
setAuthenticated(false);
window.location.href = buildLogoutHref();
}

Comment on lines +67 to +73

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking, but important: would you be amenable to us adding a backlog issue to research revoking tokens as well to ensure misconfigured or browsers on compromised systems do not "get logged out" when they think they are? This peeves me in personal contexts, and it seems AWS might support it.

https://docs.aws.amazon.com/cognito/latest/developerguide/token-revocation.html

I'd like to spike and consider this, after reviving our threat model. Thoughts? /cc @Compton-NIST

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that not what the logout endpoint does? https://docs.aws.amazon.com/cognito/latest/developerguide/logout-endpoint.html

This function redirects the user to cognito's logout URL after clearing the cookies.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not clear to me if we are doing GlobalSignOut or not and like all good web technology, the answer is almost always more complicated and "well, it depends!"

aws-amplify/amplify-js#3435 (comment)

This is why I wanted to threat model and spike accordingly, since I am a little iffy on the details, I'd like to be 100% certain and proven that I was off-base. 😆

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, I am worried about the refresh tokens since those are the focus of the revocation, but their official docs are confused by AWS devs saying how Cognito operates against API calls, so maybe I am overreacting. 🤷

async function authorize(code: string) {
console.log('Authorize called')
setLoading(true);
try {
const response = await oauthAuthorize(code);
setCookies(response.data);
setInterceptorIds(createInterceptors(refresh, setError, logout));
setAuthenticated(true);
} catch (error) {
setError(error as Error);
Expand All @@ -79,13 +87,6 @@ const AuthProvider: FC<{ children: ReactNode }> = ({ children }) => {
}
}

async function logout() {
console.log('Logout called')
clearCookies();
setAuthenticated(false);
window.location.href = buildLogoutHref();
}

// eslint-disable-next-line react-hooks/exhaustive-deps
const value = useMemo(() => ({ loading, authenticated, error, authorize, logout }), [loading, error, authenticated]);

Expand Down
2 changes: 2 additions & 0 deletions src/api/auth/instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ export function createInterceptors(
(error) => {
if (error.response.status === 401) {
refreshRetry(error.config, error);
} else {
return Promise.reject(error);
}
}
),
Expand Down
5 changes: 1 addition & 4 deletions src/api/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,5 @@ export type TransactionResponse = Record<string, string>;
export async function postTransaction(
request: TransactionRequest
): Promise<AxiosResponse<TransactionResponse>> {
return axiosAuthInstance.post("/transaction", request).then((resp) => {
resp.data = resp.data as TransactionResponse;
return resp;
});
return axiosAuthInstance.post<TransactionResponse>("/transaction", request);
}
8 changes: 3 additions & 5 deletions src/api/userinfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,11 @@ export type UserInfoResponse = {
name?: string;
given_name?: string;
family_name?: string;
preferred_username: string;
preferred_username?: string;
username: string
email: string;
};

export async function getUserInfo(): Promise<AxiosResponse<UserInfoResponse>> {
return axiosAuthInstance.get(`${AUTH_URL}/oauth2/userInfo`).then((resp) => {
resp.data = resp.data as UserInfoResponse;
return resp;
});
return axiosAuthInstance.get<UserInfoResponse>(`${AUTH_URL}/oauth2/userInfo`);
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export function FooterSimple({ links }: FooterSimpleProps) {
return (
<div className={classes.footer}>
<Container className={classes.inner}>
<Image src="./nist.png" height={105} fit="contain" />
<Image src={`${import.meta.env.BASE_URL}/nist.png`} height={105} fit="contain" />
nikitawootten-nist marked this conversation as resolved.
Show resolved Hide resolved
<Group className={classes.links}>{items}</Group>
</Container>
</div>
Expand Down
2 changes: 2 additions & 0 deletions src/components/Footer/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import { FooterSimple } from "./Footer";
export default FooterSimple;
15 changes: 8 additions & 7 deletions src/components/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,11 @@ import {
IconLogout,
IconSettings,
} from "@tabler/icons";
import { useAuth } from "api/auth";
import React from "react";
import { useState } from "react";
import LoginButton from "./LoginButton";

import { getUserInfo } from "api";
import { getUserInfo, useAuth } from "api";

const useStyles = createStyles((theme) => ({
link: {
Expand Down Expand Up @@ -88,11 +87,13 @@ export default function HeaderMegaMenu() {
const [userName, setUserName] = useState("");

React.useEffect(() => {
const getData = async () => {
const { preferred_username } = (await getUserInfo()).data;
setUserName(preferred_username);
};
getData();
if (authenticated && userName === "") {
// horribly hacky, waits until the auth interceptor has been set after being authenticated
new Promise(resolve => setTimeout(resolve, 500)).then(_ => {
getUserInfo().then(info => setUserName(info.data.username));
});
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [authenticated]);

return (
Expand Down
4 changes: 2 additions & 2 deletions src/components/Transaction/TransactionResultsDisplay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ const CodeDisplay: React.FC<{text: string, label?: string}> = ({ label, text })
sx={{
borderRadius: '5px',
background: '#002b36',
color: '#839496'
color: '#839496',
}}
>
<pre>
<code>
<code style={{whiteSpace: 'pre-line'}}>
{text}
</code>
</pre>
Expand Down
5 changes: 3 additions & 2 deletions src/components/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import LoginButton from "./LoginButton";
import FooterSimple from "./Footer";
import HeaderMegaMenu from "./Header";

export { LoginButton };
export { FooterSimple } from "./Footer";
export { LoginButton, FooterSimple as Footer, HeaderMegaMenu as Header };

export {
TransactionSelect,
Expand Down
7 changes: 3 additions & 4 deletions src/main.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ import ReactDOM from "react-dom/client";
import { HashRouter, Outlet, Route, Routes } from "react-router-dom";
import { AuthProvider, RequireAuth } from "api/auth";
import { Dashboard, Landing, NotFound, Transaction, UserInfo } from "pages";
import { FooterSimple } from "components";
import HeaderMegaMenu from "components/Header";
import { Footer, Header } from "components";

ReactDOM.createRoot(document.getElementById("root") as HTMLElement).render(
<React.StrictMode>
Expand All @@ -13,7 +12,7 @@ ReactDOM.createRoot(document.getElementById("root") as HTMLElement).render(
<div
style={{ height: "100vh", display: "flex", flexDirection: "column" }}
>
<HeaderMegaMenu />
<Header />
nikitawootten-nist marked this conversation as resolved.
Show resolved Hide resolved
<Routes>
<Route path="/" element={<Landing />} />
<Route
Expand All @@ -31,7 +30,7 @@ ReactDOM.createRoot(document.getElementById("root") as HTMLElement).render(
<Route path="*" element={<NotFound />} />
</Route>
</Routes>
<FooterSimple links={[{ link: "", label: "" }]} />
<Footer links={[{ link: "", label: "" }]} />
</div>
</AuthProvider>
</HashRouter>
Expand Down
11 changes: 6 additions & 5 deletions src/pages/Transaction.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,27 @@ import { Title, Grid, Button } from "@mantine/core";
import { postTransaction, TransactionRequest } from "api";
import { builders, TransactionResults, TransactionResultsDisplay, TransactionSelect } from "components";
import { IconClearAll } from "@tabler/icons";
import { AxiosError } from "axios";

export default function Transaction() {
const [responses, setResponses] = useState<TransactionResults[]>([])
const onSubmit = async (request: TransactionRequest) => {
try {
const response = await postTransaction(request);

// fake sleeping for now
// await new Promise(r => setTimeout(r, 1000));
// const response = {'test': 'response'};

setResponses([{
request,
response: response.data,
date: new Date()
}, ...responses]);
} catch (e) {
let message = `Client Error: ${e}`;
if (e instanceof AxiosError) {
message = `Server Error (${e.code}): ${e.response?.data}`;
} else {}
setResponses([{
request,
response: `Client Error: ${e}`,
response: message,
date: new Date()
}, ...responses])
}
Expand Down
4 changes: 3 additions & 1 deletion vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import tsconfigPaths from "vite-tsconfig-paths";
// for deployments that are deployed to a sub-folder like /dev, specify below or use the `BASE_URL` env var
const DEFAULT_BASE_URL = "/";
// to proxy transaction api requests on a different domain, specify below or use the `PROXY_URL` env var
const DEFAULT_PROXY_URL = "https://n2ft4ng3qh.execute-api.us-east-1.amazonaws.com/dev";
const DEFAULT_PROXY_URL = "https://719hgl5ejk.execute-api.us-east-1.amazonaws.com/dev";
nikitawootten-nist marked this conversation as resolved.
Show resolved Hide resolved

// https://vitejs.dev/config/
export default defineConfig({
Expand All @@ -16,6 +16,8 @@ export default defineConfig({
proxy: {
"/transaction": {
target: process.env.PROXY_URL ?? DEFAULT_PROXY_URL,
// Fixes SSL error on some requests
changeOrigin: true
}
}
}
Expand Down