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

[EASI-4486] system intake doc uploads #2691

Merged
merged 18 commits into from
Jul 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cypress/e2e/systemIntake.cy.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import cmsGovernanceTeams from '../../src/constants/enums/cmsGovernanceTeams';
import { BASIC_USER_PROD } from '../../src/constants/jobCodes';

describe('The System Intake Form', () => {
beforeEach(() => {
cy.localLogin({ name: 'E2E1' });
cy.localLogin({ name: 'E2E1', role: BASIC_USER_PROD });

cy.intercept('POST', '/api/graph/query', req => {
if (req.body.operationName === 'UpdateSystemIntakeRequestDetails') {
Expand Down
11 changes: 11 additions & 0 deletions migrations/V186__Add_Uploader_Role.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
CREATE TYPE document_uploader_role AS ENUM ('REQUESTER', 'ADMIN');

-- add column without NOT NULL
ALTER TABLE IF EXISTS system_intake_documents
ADD COLUMN IF NOT EXISTS uploader_role document_uploader_role;
-- update all rows
UPDATE system_intake_documents
SET uploader_role = 'REQUESTER';
-- add NOT NULL for new rows
ALTER TABLE IF EXISTS system_intake_documents
ALTER COLUMN uploader_role SET NOT NULL;
2 changes: 1 addition & 1 deletion pkg/graph/generated/generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

142 changes: 139 additions & 3 deletions pkg/graph/resolvers/system_intake_document.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,34 @@ package resolvers

import (
"context"
"database/sql"
"errors"
"fmt"
"path/filepath"
"slices"

"github.com/google/uuid"
"go.uber.org/zap"

"github.com/cms-enterprise/easi-app/pkg/appcontext"
"github.com/cms-enterprise/easi-app/pkg/easiencoding"
"github.com/cms-enterprise/easi-app/pkg/models"
"github.com/cms-enterprise/easi-app/pkg/services"
"github.com/cms-enterprise/easi-app/pkg/storage"
"github.com/cms-enterprise/easi-app/pkg/upload"
)

// GetSystemIntakeDocumentsByRequestID fetches all documents attached to the system intake with the given ID.
func GetSystemIntakeDocumentsByRequestID(ctx context.Context, store *storage.Store, s3Client *upload.S3Client, id uuid.UUID) ([]*models.SystemIntakeDocument, error) {
func GetSystemIntakeDocumentsByRequestID(ctx context.Context, store *storage.Store, id uuid.UUID) ([]*models.SystemIntakeDocument, error) {
mynar7 marked this conversation as resolved.
Show resolved Hide resolved
return store.GetSystemIntakeDocumentsByRequestID(ctx, id)
}

// GetURLForSystemIntakeDocument queries S3 for a presigned URL that can be used to fetch the document with the given s3Key
func GetURLForSystemIntakeDocument(s3Client *upload.S3Client, s3Key string) (string, error) {
func GetURLForSystemIntakeDocument(ctx context.Context, store *storage.Store, s3Client *upload.S3Client, s3Key string) (string, error) {
if err := canView(ctx, store, s3Key); err != nil {
return "", err
}

presignedURL, err := s3Client.NewGetPresignedURL(s3Key)
if err != nil {
return "", err
Expand Down Expand Up @@ -48,6 +57,20 @@ func GetStatusForSystemIntakeDocument(s3Client *upload.S3Client, s3Key string) (

// CreateSystemIntakeDocument uploads a document to S3, then saves its metadata to our database.
func CreateSystemIntakeDocument(ctx context.Context, store *storage.Store, s3Client *upload.S3Client, input models.CreateSystemIntakeDocumentInput) (*models.SystemIntakeDocument, error) {
intake, err := store.FetchSystemIntakeByID(ctx, input.RequestID)
if err != nil {
return nil, err
}

uploaderRole, err := getUploaderRole(ctx, intake)
if err != nil {
return nil, err
}

if err := canCreate(ctx, uploaderRole, intake); err != nil {
return nil, err
}

s3Key := uuid.New().String()

existingExtension := filepath.Ext(input.FileData.Filename)
Expand All @@ -73,6 +96,7 @@ func CreateSystemIntakeDocument(ctx context.Context, store *storage.Store, s3Cli
FileName: input.FileData.Filename,
S3Key: s3Key,
Bucket: s3Client.GetBucket(),
UploaderRole: uploaderRole,
// Status isn't saved in database - will be fetched from S3
// URL isn't saved in database - will be generated by querying S3
}
Expand All @@ -84,9 +108,121 @@ func CreateSystemIntakeDocument(ctx context.Context, store *storage.Store, s3Cli
return store.CreateSystemIntakeDocument(ctx, &documentDatabaseRecord)
}

// DeleteSystemIntakeDocument deletes an existing SystemIntakeDocumet, given its ID.
// DeleteSystemIntakeDocument deletes an existing SystemIntakeDocument, given its ID.
//
// Does *not* delete the uploaded file from S3, following the example of TRB request documents.
func DeleteSystemIntakeDocument(ctx context.Context, store *storage.Store, id uuid.UUID) (*models.SystemIntakeDocument, error) {
if err := canDelete(ctx, store, id); err != nil {
return nil, err
}

return store.DeleteSystemIntakeDocument(ctx, id)
}

// canView guards unauthorized views (downloads) of system intake documents
// admins can view any document
// requesters can view any document
// GRB reviewers can view any document
func canView(ctx context.Context, store *storage.Store, s3Key string) error {
// admins can view
if services.HasRole(ctx, models.RoleEasiGovteam) {
return nil
}

document, err := store.GetSystemIntakeDocumentByS3Key(ctx, s3Key)
if err != nil {

if errors.Is(err, sql.ErrNoRows) {
// if the document was deleted, don't error and break
return nil
}
Comment on lines +135 to +138
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adamodd if you were curious what that issue was. it wasn't on the delete at all. finally realized this morning that it was err no rows on the URL resolver for a system intake document, and that runs after deletion. in main, we go straight to S3 for that URL, but in this auth patch, we first check the DB to see who created the document, but it is gone after deletion!


return err
}

// requester can view
user := appcontext.Principal(ctx).Account()
if document.GetCreatedBy() == user.Username {
return nil
}

// if needed, get GRB members for this intake
grbUsers, err := store.SystemIntakeGRBReviewersBySystemIntakeIDs(ctx, []uuid.UUID{document.SystemIntakeRequestID})
if err != nil {
return err
}

if isGRBViewer := slices.ContainsFunc(grbUsers, func(reviewer *models.SystemIntakeGRBReviewer) bool {
return reviewer.UserID == user.ID
}); isGRBViewer {
return nil
}

appcontext.ZLogger(ctx).Warn("unauthorized user attempted to view system intake document",
zap.String("username", user.Username),
zap.String("system_intake.id", document.SystemIntakeRequestID.String()),
zap.String("document.id", document.ID.String()))

return errors.New("unauthorized attempt to view system intake document")
}

// getUploaderRole guards unauthorized creation of system intake documents
// admins can upload documents
// requesters can upload documents
func getUploaderRole(ctx context.Context, intake *models.SystemIntake) (models.DocumentUploaderRole, error) {
// check requester first as that role takes precedence over admin role for uploader roles
user := appcontext.Principal(ctx).Account()
if intake.EUAUserID.String == user.Username {
return models.RequesterUploaderRole, nil
}

if services.HasRole(ctx, models.RoleEasiGovteam) {
return models.AdminUploaderRole, nil
}

appcontext.ZLogger(ctx).Warn("unauthorized user attempted to create system intake document",
zap.String("username", user.Username),
zap.String("system_intake.id", intake.ID.String()))

return "", errors.New("unauthorized attempt to create system intake document")
}

func canCreate(ctx context.Context, role models.DocumentUploaderRole, intake *models.SystemIntake) error {
if role == models.RequesterUploaderRole || role == models.AdminUploaderRole {
return nil
}

appcontext.ZLogger(ctx).Warn("unauthorized user attempted to create system intake document",
zap.String("username", appcontext.Principal(ctx).Account().Username),
zap.String("system_intake.id", intake.ID.String()))

return errors.New("unauthorized attempt to create system intake document")
}

// canDelete guards unauthorized deletions of system intake documents
// an admin is allowed to delete admin-uploaded docs
// the intake requester is allowed to delete requester-uploaded docs
func canDelete(ctx context.Context, store *storage.Store, id uuid.UUID) error {
document, err := store.GetSystemIntakeDocumentByID(ctx, id)
if err != nil {
return err
}

// admins can only delete admin-uploaded docs
if services.HasRole(ctx, models.RoleEasiGovteam) && document.UploaderRole == models.AdminUploaderRole {
return nil
}

// if the acting user is the requester, they can delete the doc
user := appcontext.Principal(ctx).Account()
if document.GetCreatedBy() == user.Username {
return nil
}

appcontext.ZLogger(ctx).Warn("unauthorized user attempted to delete system intake document",
zap.String("username", user.Username),
zap.String("system_intake.id", document.SystemIntakeRequestID.String()),
zap.String("document.id", document.ID.String()))

return errors.New("unauthorized attempt to delete system intake document")
}
18 changes: 4 additions & 14 deletions pkg/graph/resolvers/system_intake_document_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
func (s *ResolverSuite) TestSystemIntakeDocumentResolvers() {
ctx := s.testConfigs.Context
store := s.testConfigs.Store
s3Client := s.testConfigs.S3Client

// Create a system intake
intake, err := store.CreateSystemIntake(ctx, &models.SystemIntake{
Expand All @@ -23,7 +22,7 @@ func (s *ResolverSuite) TestSystemIntakeDocumentResolvers() {
s.NotNil(intake)

// Check that there are no docs by default
docs, err := GetSystemIntakeDocumentsByRequestID(ctx, store, s3Client, intake.ID)
docs, err := GetSystemIntakeDocumentsByRequestID(ctx, store, intake.ID)
s.NoError(err)
s.Len(docs, 0)

Expand All @@ -34,6 +33,7 @@ func (s *ResolverSuite) TestSystemIntakeDocumentResolvers() {
FileName: "create_and_get.pdf",
Bucket: "bukkit",
S3Key: uuid.NewString(),
UploaderRole: models.RequesterUploaderRole,
}
// documentToCreate.CreatedBy will be set based on principal in test config

Expand Down Expand Up @@ -75,12 +75,7 @@ func createSystemIntakeDocumentSubtest(s *ResolverSuite, systemIntakeID uuid.UUI
}

func getSystemIntakeDocumentsByRequestIDSubtest(s *ResolverSuite, systemIntakeID uuid.UUID, createdDocument *models.SystemIntakeDocument) {
documents, err := GetSystemIntakeDocumentsByRequestID(
s.testConfigs.Context,
s.testConfigs.Store,
s.testConfigs.S3Client,
systemIntakeID,
)
documents, err := GetSystemIntakeDocumentsByRequestID(s.testConfigs.Context, s.testConfigs.Store, systemIntakeID)
s.NoError(err)
s.Equal(1, len(documents))

Expand All @@ -96,12 +91,7 @@ func deleteSystemIntakeDocumentSubtest(s *ResolverSuite, createdDocument *models
s.NoError(err)
checkSystemIntakeDocumentEquality(s, createdDocument, createdDocument.CreatedBy, createdDocument.SystemIntakeRequestID, deletedDocument)

remainingDocuments, err := GetSystemIntakeDocumentsByRequestID(
s.testConfigs.Context,
s.testConfigs.Store,
s.testConfigs.S3Client,
createdDocument.SystemIntakeRequestID,
)
remainingDocuments, err := GetSystemIntakeDocumentsByRequestID(s.testConfigs.Context, s.testConfigs.Store, createdDocument.SystemIntakeRequestID)
s.NoError(err)
s.Equal(0, len(remainingDocuments))
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/graph/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -2610,7 +2610,7 @@ A user role associated with a job code
"""
enum Role {
"""
A member of the GRT
An admin on the GRT
"""
EASI_GOVTEAM

Expand Down
4 changes: 2 additions & 2 deletions pkg/graph/schema.resolvers.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/models/models_gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions pkg/models/system_intake_document.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ type SystemIntakeDocumentStatus string
// SystemIntakeDocumentCommonType represents the document type, including an "OTHER" option for user-specified types
type SystemIntakeDocumentCommonType string

type DocumentUploaderRole string

const (
// SystemIntakeDocumentStatusAvailable means that the document passed the virus scanning
SystemIntakeDocumentStatusAvailable SystemIntakeDocumentStatus = "AVAILABLE"
Expand All @@ -22,6 +24,11 @@ const (
SystemIntakeDocumentCommonTypeDraftICGE SystemIntakeDocumentCommonType = "DRAFT_ICGE"
// SystemIntakeDocumentCommonTypeDraftOther means the document is some type other than the common document types
SystemIntakeDocumentCommonTypeDraftOther SystemIntakeDocumentCommonType = "OTHER"

// RequesterUploaderRole signifies a Requester uploaded a document
RequesterUploaderRole DocumentUploaderRole = "REQUESTER"
// AdminUploaderRole signifies an Admin uploaded a document
AdminUploaderRole DocumentUploaderRole = "ADMIN"
)

// SystemIntakeDocument represents a document attached to a system intake that has been uploaded to S3
Expand All @@ -33,4 +40,5 @@ type SystemIntakeDocument struct {
FileName string `json:"fileName" db:"file_name"`
Bucket string `json:"bucket" db:"bucket"`
S3Key string `json:"s3Key" db:"s3_key"` // The document's key inside an S3 bucket; does *not* include the bucket name.
UploaderRole DocumentUploaderRole `json:"uploaderRole" db:"uploader_role"`
}
33 changes: 33 additions & 0 deletions pkg/sqlqueries/SQL/system_intake_document/create.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
INSERT INTO system_intake_documents (id,
system_intake_id,
file_name,
document_type,
other_type,
bucket,
s3_key,
created_by,
modified_by,
uploader_role)
VALUES (:id,
:system_intake_id,
:file_name,
:document_type,
:other_type,
:bucket,
:s3_key,
:created_by,
:modified_by,
:uploader_role)
RETURNING
id,
system_intake_id,
file_name,
document_type,
other_type,
bucket,
s3_key,
created_by,
created_at,
modified_by,
modified_at,
uploader_role;
16 changes: 16 additions & 0 deletions pkg/sqlqueries/SQL/system_intake_document/delete.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
DELETE
FROM system_intake_documents
WHERE id = :id
RETURNING
id,
system_intake_id,
file_name,
document_type,
other_type,
bucket,
s3_key,
created_by,
created_at,
modified_by,
modified_at,
uploader_role;
Loading