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

Backend/309/final #121

Merged
merged 73 commits into from
Jan 12, 2021
Merged

Backend/309/final #121

merged 73 commits into from
Jan 12, 2021

Conversation

carltonsmith
Copy link
Contributor

@carltonsmith carltonsmith commented Jan 8, 2021

This pull request changes...

Adds functionality for supporting roles and permissions, as well as Django Admin abilities to manage users, groups and permissions. The user auth_check now returns the user's roles and associated permissions.

TO TEST

Our recommendation is for the Raft tech lead @carltonsmith to demonstrate the capabilities as this backend centered pull request can be rather technical to test. Here are the changes that can be demonstrated as a result of this pull request.

  • After logging in as a privileged user, using a Rest Client browser extension, polling the previously existing auth_check endpoint now returns the logged in user's assigned roles and permissions.
  • A new endpoint exists to list the existing roles in the system at /v1/roles/ (This satisfies the AC "Preliminary (MVP) roles are populated in the database")
    Security Control Requirement Changes
    These changes were made to address security control requirements
  • A user with built-in Django roles Staff and SuperUser is able to log in to the Django Admin console
  • In the Django Admin console all users' data can be updated including first_name, last_name, email and assigned roles (called Groups in the Django Admin)
  • Groups can be managed by updating permissions for each group
  • All changes made in the Django Admin Console are logged and viewable within the console

This pull request is ready to merge when...

  • Meets acceptance criteria for the issue
  • Code is reviewed by someone other than the original author
  • Code is tested
  • Experience is approved by UX
  • Meets a11y checklist
  • Meets Raft's Manual QASP Checklist🔒 (only applicable for PR to main HHS)
  • Documentation is updated
    • OpenAPI
    • Readme
    • Entity Relationship Diagram (ERD)

@carltonsmith
Copy link
Contributor Author

Quick note on the size of this PR, as our current strategy is to aim for much slimmer PRs:

The AC of 309 was to simply demonstrate that the roles are in the system, however, to include roles in the system, the architecture to support roles and permissions needs to be included. Then, the determination was made that we don't want to use the CLI to update users because there is no audit trail, and it was decided that that needed to be part of this PR because it would also satisfy the requirement of demonstrating what roles are in the system.

@carltonsmith carltonsmith added the QASP Review This is ready for QASP review label Jan 8, 2021
@alexsoble
Copy link
Contributor

QASP review

This review covers all deliverables except Deliverable 1 (Accepted Features), which we will review in a separate meeting, and Deliverable 4 (Accessible), which will likely require accessibility testing by @iamjolly and @ttran-hub next week.

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:

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? — Yes, see API tests under tdrs-backend/tdpservice/users/test/test_api/
  • Are code coverage minimums met?
    • Frontend coverage: 100%
    • Backend coverage: 100%

Deliverable 3: Properly Styled Code

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

Acceptable Quality Level: 0 linting errors and 0 warnings

  • Backend code style checks passing on CircleCI
  • Frontend code style checks passing on CircleCI

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

  • 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 build with a single command

  • Code successfully deployed via automated CircleCI process to development on Cloud.gov

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?

Yes, see:

  • tdrs-backend/docs/api/roles.md
  • docs/Technical-Documentation/user_role_management.md

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

  • OWASP ZAP Scan passes on CircleCI

Manual code review detected three areas for security improvement, which have been documented and will be prioritized to follow this PR:

  1. As a super user, I would like my access to the admin dashboard protected by two-factor authentication: https://app.zenhub.com/workspaces/tdrs-product-backlog-5f2c6cdc7c0bb1001bdc43a5/issues/raft-tech/tanf-app/517undefined 2768 zap fix #517
  2. Remove API endpoint for creating new users: https://app.zenhub.com/workspaces/tdrs-product-backlog-5f2c6cdc7c0bb1001bdc43a5/issues/raft-tech/tanf-app/518undefined Create sprint-97-summary.md #518
  3. As a dev, I would like to be able to assign super user status without SSH: https://app.zenhub.com/workspaces/tdrs-product-backlog-5f2c6cdc7c0bb1001bdc43a5/issues/raft-tech/tanf-app/519

Copy link
Collaborator

@lfrohlich lfrohlich left a comment

Choose a reason for hiding this comment

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

@carltonsmith demoed the functionality on 1/11/21 and it does what it says it does. looks good!

@lfrohlich
Copy link
Collaborator

lfrohlich commented Jan 11, 2021

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

  • Did automated and manual testing with @iamjolly and @ttran-hub using Accessibility Insights reveal any errors introduced in this PR?

Given the backend nature and limited users who will the django admin console, we plan to defer the accessibility review for a later sprint. Is that ok with you, @iamjolly and @ttran-hub ? I created a follow up issue here: https://app.zenhub.com/workspaces/tdrs-sprint-board-5f18ab06dfd91c000f7e682e/issues/raft-tech/tanf-app/528

@iamjolly
Copy link
Collaborator

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

  • Did automated and manual testing with @iamjolly and @ttran-hub using Accessibility Insights reveal any errors introduced in this PR?

Given the backend nature and limited users who will the django admin console, we plan to defer the accessibility review for a later sprint. Is that ok with you, @iamjolly and @ttran-hub ? I created a follow up issue here: https://app.zenhub.com/workspaces/tdrs-sprint-board-5f18ab06dfd91c000f7e682e/issues/raft-tech/tanf-app/528

@lfrohlich - That sounds fine to me.

@lfrohlich lfrohlich merged commit cc9fad4 into HHS:main Jan 12, 2021
@carltonsmith carltonsmith deleted the backend/309/final branch January 12, 2021 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
QASP Review This is ready for QASP review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants