-
-
Notifications
You must be signed in to change notification settings - Fork 33
Bugfix/484/enable single sign on for react app through people depot #485
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
base: main
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
commit 0d70d8f Author: Ethan Strominger <ethanstrominger2@gmail.com> Date: Wed Dec 18 11:42:21 2024 -0500 Update graphviz commit 775d753 Author: Ethan Strominger <ethanstrominger2@gmail.com> Date: Sun Dec 1 17:25:56 2024 -0500 Modify to tricker pre-commit commit c75db16 Author: Ethan Strominger <ethanstrominger2@gmail.com> Date: Sun Dec 1 17:23:16 2024 -0500 Fix trailing whitespace
commit 0d70d8f Author: Ethan Strominger <ethanstrominger2@gmail.com> Date: Wed Dec 18 11:42:21 2024 -0500 Update graphviz commit 775d753 Author: Ethan Strominger <ethanstrominger2@gmail.com> Date: Sun Dec 1 17:25:56 2024 -0500 Modify to tricker pre-commit commit c75db16 Author: Ethan Strominger <ethanstrominger2@gmail.com> Date: Sun Dec 1 17:23:16 2024 -0500 Fix trailing whitespace
…pp-through-People-Depot
…pp-through-People-Depot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see you're adding cors to allow a frontend to connect from a different domain, which is good.
I see a potential problem as well:
rest_framework.authtokenadds a separate token-based login method in addition to the one provided by cognito. I don't think it's something we want to do.
|
@ethanstrominger is there a good way to test/validate this PR? |
|
This PR is not testable until the following issue is done: Converting it to draft for now. |
Fixes #484
What changes did you make?
Dockerfile - updated graphiz versionChanged core.utils.jwt to core.utils.jwt_handler to disambiguate from the cors packageWhy did you make the changes?
Testing