-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat(1136): Accept AMS OpenID #1369
Conversation
c4efc37
to
3dae31d
Compare
6d58ac7
to
a061431
Compare
…into feat/1136-ams
ccd8c3e
to
9cec3e3
Compare
…into feat/1136-ams
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.
BIG MILESTONE! 🚀 thank you @jorgegonzalez
Some important notes:
- i updated ams env var/val pairs in cloud.gov for qasp and staging envs to complete the review for this work.
- noted below that I had to set accurate the env values to test locally. we should have a note about this saved somewhere centrally for future ref.
- we're limited to one url right now for the configuration, so as of today, this will only work on the raft deployed site (
https://tdp-frontend-raft.app.cloud.gov/
) @lfrohlich
ready to merge @abottoms-coder
@jorgegonzalez Do we have a follow-on task for updating AMS to allow |
Not that I know of, I defer to you and @ADPennington for the appropriate response there |
i think there is a feasibility question here for ACF AMS Ops. We can track that discussion in a spike ticket that links to the acf ams epic. @abottoms-coder @jorgegonzalez @valcollignon |
This is now to be tracked in #1552 |
Summary of Changes
This PR adds initial support in the backend for accepting AMS OpenID As an authentication provider.
Closes #1136
Acceptance criteria as stated in the issue
How to Test
List the steps to test the PR
These steps are generic, please adjust as necessary.
http://localhost:3000/
orhttps://tdp-frontend-raft.app.cloud.gov
and sign in using the Login With AMS button.IMPORTANT NOTE: given limited acf ams config settings, only one deployed site url (the one above) currently works. we'll inquire if this can change but for now, this will NOT work in staging env
Deliverable 1: Accepted Features
also confirmed that external user route still works
https://tdp-backend-raft.app.cloud.gov/admin/
for logged out user re-routes to frontend (addressing As technical lead, I need ACF AMS integrated for direct DAC access #1520 behavior)As Product Owner, @lfrohlich will decide if ACs are met.
Deliverable 2: Tested Code
Deliverable 3: Properly Styled Code
Deliverable 4: Accessible
n/a: this PR will redirect users to ACF AMS for authentication. ACF AMS meets compliance standards.
Deliverable 5: Deployed
yes, BUT, as noted above, this only works on the raft deployed site, given configuration limitations beyond TDP team's control.
cc: @lfrohlich @abottoms-coder
Deliverable 6: Documented
https://pypi.org/project/requests-mock/
Deliverable 7: Secure
no new issues detected
Deliverable 8: Context