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

feat(1136): Accept AMS OpenID #1369

Merged
merged 87 commits into from
Jan 18, 2022
Merged

feat(1136): Accept AMS OpenID #1369

merged 87 commits into from
Jan 18, 2022

Conversation

jorgegonzalez
Copy link

@jorgegonzalez jorgegonzalez commented Oct 7, 2021

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.

cd tdrs-frontend && docker-compose -f docker-compose.yml -f docker-compose.local.yml up -d
cd tdrs-backend && docker-compose -f docker-compose.yml -f docker-compose.local.yml up -d 
  1. Open http://localhost:3000/ or https://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
  2. Proceed with the login flow in HHS AMS.
  3. Ensure that you are redirected to TDRS
  4. Ensure you are able to use the app as normal
  5. Log out, and get redirected to the main login page with no errors.
  6. Test steps should be captured in the demo GIF(s) and/or screenshots below.

Demo GIF(s) and screenshots for testing procedure

Deliverable 1: Accepted Features

Performance Standard(s): At the beginning of each sprint, the Product Owner and development team will collaborate to define a set of user stories to be completed during the sprint. Acceptance criteria for each story will also be defined. The development team will deliver code and functionality to satisfy these user stories.

Acceptable Quality Level: Delivered code meets the acceptance criteria for each user story. Incomplete stories will be assessed and considered for inclusion in the next sprint.

  • Look up the acceptance criteria in the related issue; paste ACs below in checklist format.
  • Check against the criteria:
  • Authentication and Authorization can pass through successfully using AMS.
  • User's request is redirected back to our frontend
  • Testing Checklist has been run and all tests pass
    also confirmed that external user route still works
  • README is updated, if necessary
  • 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)
  • @ADPennington reviewed and tested

As Product Owner, @lfrohlich will decide if ACs are met.

Deliverable 2: Tested Code

Performance Standard(s): Code delivered under the order must have substantial test code coverage. Version-controlled HHS GitHub repository of code that comprises products that will remain in the government domain.

Acceptable Quality Level: Minimum of 90% test coverage of all code. All areas of code are meaningfully tested.

  • Are all areas of code introduced in this PR meaningfully tested?
    • If this PR introduces backend code changes, are they meaningfully tested?
    • If this PR introduces frontend code changes, are they meaningfully tested?
  • Are code coverage minimums met?

Deliverable 3: Properly Styled Code

Performance Standard(s): GSA 18F Front- End Guide

Acceptable Quality Level: 0 linting errors and 0 warnings

  • Are backend code style checks passing on CircleCI?
  • Are frontend code style checks passing on CircleCI?
  • Does this PR change any linting or CI settings?

Deliverable 4: Accessible

Performance Standard(s): Web Content Accessibility Guidelines 2.1 AA standards

Acceptable Quality Level: 0 errors reported using an automated scanner and 0 errors reported in manual testing

  • Does this PR complete the epic?
  • Are links included to any other gov-approved PRs associated with epic?
  • Does PR include documentation for Raft's a11y review?
  • Did automated and manual testing with @iamjolly and @ttran-hub using Accessibility Insights reveal any errors introduced in this PR?

Deliverable 5: Deployed

Performance Standard(s): Code must successfully build and deploy into the staging environment.

Acceptable Quality Level: Successful deployment by assigning a 'Deploy with CircleCI - {env}' label

  • Was the code successfully deployed via automated CircleCI process to development on Cloud.gov?
    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

Performance Standard(s): Summary of user stories completed every two weeks. All dependencies are listed and the licenses are documented. Major functionality in the software/source code is documented, including system diagram. Individual methods are documented inline in a format that permits the use of tools such as JSDoc. All non-inherited 800-53 system security controls are documented in the Open Control or OSCAL format and HHS Section 508 Product Assessment Template (PAT) are updated as appropriate.

Acceptable Quality Level: Combination of manual review and automated testing, if available

  • If this PR introduces backend code, is that code documented both inline and overall?
  • If this PR introduces frontend code, is that code documented both inline and overall?
  • If this PR introduces dependencies, are their licenses documented?
    https://pypi.org/project/requests-mock/

Deliverable 7: Secure

Performance Standard(s): Open Web Application Security Project (OWASP) Application Security Verification Standard 3.0

Acceptable Quality Level: Code submitted must be free of medium- and high-level static and dynamic security vulnerabilities

  • Does the OWASP Scan pass on CircleCI?
  • Do manual code review and manual testing detect any security issues?
    no new issues detected

Deliverable 8: Context

  • Performance Standard(s): Code must be understandable and contextualized for the reviewers possess the knowledge and background necessary for analysis and constructive criticism to take place.
  • Acceptable Quality Level: Code submitted in the pull request has context.*
  • Does this pull request contain sufficient inline comments providing relevant context for code snippets?
  • Is the code understandable and lucid?
  • Does this pull request provide background for why coding decisions were made?
  • Can you as a reviewer explain and take ownership of these elements presented in this code review?

@jorgegonzalez jorgegonzalez changed the title [WIP] Issue 1135: Accept AMS OpenID [WIP] Issue 1136: Accept AMS OpenID Oct 7, 2021
@jorgegonzalez jorgegonzalez linked an issue Oct 7, 2021 that may be closed by this pull request
10 tasks
@ADPennington ADPennington added Deploy with CircleCI-qasp Deploy to https://tdp-frontend-qasp.app.cloud.gov through CircleCI Deploy with CircleCI-raft Deploy to https://tdp-frontend-raft.app.cloud.gov through CircleCI and removed Deploy with CircleCI-qasp Deploy to https://tdp-frontend-qasp.app.cloud.gov through CircleCI Deploy with CircleCI-raft Deploy to https://tdp-frontend-raft.app.cloud.gov through CircleCI labels Jan 18, 2022
@jorgegonzalez jorgegonzalez removed the Deploy with CircleCI-raft Deploy to https://tdp-frontend-raft.app.cloud.gov through CircleCI label Jan 18, 2022
@ADPennington ADPennington added the Deploy with CircleCI-qasp Deploy to https://tdp-frontend-qasp.app.cloud.gov through CircleCI label Jan 18, 2022
@ADPennington ADPennington added Deploy with CircleCI-qasp Deploy to https://tdp-frontend-qasp.app.cloud.gov through CircleCI and removed Deploy with CircleCI-qasp Deploy to https://tdp-frontend-qasp.app.cloud.gov through CircleCI labels Jan 18, 2022
@ADPennington ADPennington added Deploy with CircleCI-qasp Deploy to https://tdp-frontend-qasp.app.cloud.gov through CircleCI Deploy with CircleCI-raft Deploy to https://tdp-frontend-raft.app.cloud.gov through CircleCI and removed Deploy with CircleCI-qasp Deploy to https://tdp-frontend-qasp.app.cloud.gov through CircleCI labels Jan 18, 2022
Copy link
Collaborator

@ADPennington ADPennington left a 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

@andrew-jameson
Copy link
Collaborator

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 -staging or were we going to wait for the prod URL?

@andrew-jameson andrew-jameson merged commit 11fd810 into raft-tdp-main Jan 18, 2022
@andrew-jameson andrew-jameson deleted the feat/1136-ams branch January 18, 2022 17:10
@jorgegonzalez
Copy link
Author

@jorgegonzalez Do we have a follow-on task for updating AMS to allow -staging or were we going to wait for the prod URL?

Not that I know of, I defer to you and @ADPennington for the appropriate response there

@ADPennington
Copy link
Collaborator

@jorgegonzalez Do we have a follow-on task for updating AMS to allow -staging or were we going to wait for the prod URL?

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

@andrew-jameson
Copy link
Collaborator

@jorgegonzalez Do we have a follow-on task for updating AMS to allow -staging or were we going to wait for the prod URL?

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
authentication authentication backend Deploy with CircleCI-raft Deploy to https://tdp-frontend-raft.app.cloud.gov through CircleCI dev Ready to Merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Backend] Accept AMS OpenId
5 participants