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

Conversation

nikitawootten-nist
Copy link
Contributor

Fixes #30 and Fixes #29

  • Vite proxy handles transaction requests properly
  • Static resources (nist logo) no longer fails to load
  • Bad requests no longer fail
  • Cleanup

Fixes #30 and Fixes #29

- Vite proxy handles transaction requests properly
- Static resources (nist logo) no longer fails to load
- Bad requests no longer fail
- Cleanup
Copy link

@aj-stein-nist aj-stein-nist left a comment

Choose a reason for hiding this comment

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

Mostly minor feedback would like to further pursue outside this PR spike and potential implementation of going beyond just cookie deletion for session management if possible.

vite.config.ts Outdated Show resolved Hide resolved
src/main.tsx Show resolved Hide resolved
src/components/Footer/Footer.tsx Outdated Show resolved Hide resolved
Comment on lines +67 to +73
async function logout() {
console.log('Logout called')
clearCookies();
setAuthenticated(false);
window.location.href = buildLogoutHref();
}

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. 🤷

@nikitawootten-nist nikitawootten-nist merged commit 2104389 into main Nov 2, 2022
@nikitawootten-nist nikitawootten-nist deleted the nikitawootten-nist/issue30 branch November 2, 2022 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants