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

Require auth for saving and loading projects #124

Merged
merged 6 commits into from
May 25, 2021
Merged

Conversation

aerlaut
Copy link
Contributor

@aerlaut aerlaut commented May 24, 2021

Background

Link to issue

https://biomage.atlassian.net/browse/BIOMAGE-1006

Link to staging deployment URL

In UI repo : hms-dbmi-cellenics/ui#267

Links to any Pull Requests related to this

UI : hms-dbmi-cellenics/ui#267
API : #124

Anything else the reviewers should know about the changes here

  • Add 401 errors around paths listing projects
  • Add 401 and 403 errors around paths creating and updating projects

Changes

Code changes

Definition of DONE

Your changes will be ready for merging after each of the steps below have been completed:

Testing

  • [] Unit tests written
  • Tested locally with Inframock (with latest production data downloaded with biomage experiment pull)
  • Deployed to staging

To set up easy local testing with inframock, follow the instructions here: https://github.com/biomage-ltd/inframock
To deploy to the staging environment, follow the instructions here: https://github.com/biomage-ltd/biomage-utils

Documentation updates

Is all relevant documentation updated to reflect the proposed changes in this PR?

  • Relevant Github READMEs updated
  • Relevant wiki pages created/updated

Approvers

  • Approved by a member of the core engineering team
  • (UX changes) Approved by vickymorrison (this is her username, tag her if you need approval)

Just before merging:

Optional

  • Photo of a cute animal attached to this PR

src/api/routes/projects.js Show resolved Hide resolved
@aerlaut aerlaut requested a review from xverges May 25, 2021 14:38
Copy link
Contributor

@xverges xverges left a comment

Choose a reason for hiding this comment

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

I think that there are still two endpoints with the wrong middleware, can you verify?

src/utils/authMiddlewares.js Show resolved Hide resolved
src/api/routes/projects.js Show resolved Hide resolved
src/api/routes/projects.js Show resolved Hide resolved
@aerlaut aerlaut merged commit 918a69b into develop May 25, 2021
@aerlaut aerlaut deleted the projects-require-auth branch May 25, 2021 16:54
kafkasl added a commit that referenced this pull request Jun 15, 2021
* Fixed configs for develop (#106)

* change default-config values for new staging develop env

* Bump hosted-git-info from 2.8.8 to 2.8.9 (#108)

Bumps [hosted-git-info](https://github.com/npm/hosted-git-info) from 2.8.8 to 2.8.9.
- [Release notes](https://github.com/npm/hosted-git-info/releases)
- [Changelog](https://github.com/npm/hosted-git-info/blob/v2.8.9/CHANGELOG.md)
- [Commits](npm/hosted-git-info@v2.8.8...v2.8.9)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump lodash from 4.17.19 to 4.17.21 (#104)

Bumps [lodash](https://github.com/lodash/lodash) from 4.17.19 to 4.17.21.
- [Release notes](https://github.com/lodash/lodash/releases)
- [Commits](lodash/lodash@4.17.19...4.17.21)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Get data upload status (#105)

* create projects and samples
* update projects and samples
* delete projects and samples

* Add FastMNN and No integration methods (#107)

* Worker failing to restart (#111)

* Debug logging on helm command that launches worker

* Get helm status

* Dsiplay helm history on upgrade failure

* Do not run helm update concurrently

* fix async-lock

* Cleanup

* Better logging

* Run npm audit fix because dependabot is complaining again (#115)

* Launch new experiment (#113)

* Store new experiment in DynamoDB

* Add type to data processing report (#117)

Co-authored-by: Martin Fosco <[email protected]>

* Add end-to-end authentication and authorization (#110)

* Add authentication check for API

* fix

* Add authorization to the API

* add authorization to socketio

* fix existing unit tests and add a new type of test

* Add tests for authentication

* fix-typo

* Fix documentation

* learn to spell unauthenticated

* debug

* more-debug

* Suppress missing X-Ray context errors for auth middleware lookups

* fix-error

* Fix canWrite type issues

* Throw if experiment does not have rbac_can_write

* Fix broken merge

* Remove Xray noisy log

* Search for noisy Xray log

* Try to convince setContextMissingStrategy not to log

* Fix non-context Xray traces

* Remove debugging logs

* Move some commenst from PR to code

Co-authored-by: Martin Fosco <[email protected]>
Co-authored-by: Xavier Verges <[email protected]>

* Add cognito user and admin details to rbac can write (#119)

* Add authentication check for API

* fix

* Add authorization to the API

* add authorization to socketio

* fix existing unit tests and add a new type of test

* Add tests for authentication

* fix-typo

* Fix documentation

* learn to spell unauthenticated

* debug

* more-debug

* Suppress missing X-Ray context errors for auth middleware lookups

* fix-error

* Fix canWrite type issues

* Throw if experiment does not have rbac_can_write

* Fix broken merge

* Add create experiment endpoint to avoid error message experiment not found  when creating new experiment

* Add user permissions to saved experiment

* Add adminArn

* Clear out repetitions before setting rbac_can_write

* Remove extra console logs

* Test fixes

* Add 401 and 403 responses to enabled options

* Rename new auth middleware

* Move adminArn into config

Co-authored-by: Marcell Pek <[email protected]>
Co-authored-by: Marcell Pék <[email protected]>
Co-authored-by: Martin Fosco <[email protected]>
Co-authored-by: Xavier Verges <[email protected]>

* load projects and samples (#114)

* redoing the ticket

* fix

* fixes

* fixes

* comment fixe

* fixes

* move to inside function

* change

* Refactor code to work better with async/await

* Make sure keys returned to getProjectsFromIds are unqiue.

* Convert projectIds into an Array

* Fix conversion error between keys

* Add array spreading to other reference to projectIds

* Add array spreading to other references to projectIds

* Make module to filter out orphaned experiments more explicit

* Remove based on items

Co-authored-by: Marcell Pek <[email protected]>

* Recover settings accidentally removed (#121)

* api to load experiments (#126)

* created gem2s endpoint (#109)

* created gem2s endpoint

* added skeleton parameter to state-machine-definition tests

* passing all required params

* fixed qc-pipeline skeleton

* fixed gem2s pipeline skeleton name

* renamed ARN to avoid clashes

* converted gem2s pipeline steps into normal ones instead of maps

* added last steps for gem2s

* removed testing clutter

* fix manifest grep

* fixed some tests

* using personal IAC to bypass runner name temporarily

* added debug logs

* removed old comment

* added temporarily pipeline config until the updated one is deployed to iac

* added authorization to new endpoints

Co-authored-by: Alex Pickering <[email protected]>
Co-authored-by: Martin Fosco <[email protected]>

* Address comments pr 126 (#128)

* add route test
* add test for getListOfExperiments

* Save gem2s pipeline handle (#120)

* created gem2s endpoint

* added skeleton parameter to state-machine-definition tests

* passing all required params

* fixed qc-pipeline skeleton

* fixed gem2s pipeline skeleton name

* renamed ARN to avoid clashes

* converted gem2s pipeline steps into normal ones instead of maps

* added last steps for gem2s

* removed testing clutter

* fixed some tests

* Save gem2s pipeline handle

Co-authored-by: Pol Alvarez <[email protected]>
Co-authored-by: Martin Fosco <[email protected]>

* Get projects test (#123)

* test for projects route
* write tests for projects
* rewrite AWS mock

* Handle gem2s response (#127)

* created gem2s endpoint

* added skeleton parameter to state-machine-definition tests

* passing all required params

* fixed qc-pipeline skeleton

* fixed gem2s pipeline skeleton name

* renamed ARN to avoid clashes

* converted gem2s pipeline steps into normal ones instead of maps

* added last steps for gem2s

* removed testing clutter

* fixed some tests

* Save gem2s pipeline handle

* Save response to dynamodb

* Add gem2s handle and status report to status

* Fix dynamodb updates to experiments and other minor things

* Fix tests and minor bugs

* Minor fixes

Co-authored-by: Pol Alvarez <[email protected]>
Co-authored-by: Martin Fosco <[email protected]>

* Require auth for saving and loading projects (#124)

* require auth for saving and loading projects
* api to load experiments
* add auth to routes

* Request GEM2S status from the backend (#118)

* add gem2s when requesting backend status
* create routes, add tests
* change from pipelines to backend-status
* remove package-lock.json

* Delete project delete experiment [BIOMAGE-948] (#131)

* delete experiments
* add experimentDelete test
* add checks for empty experiment

* BIOMAGE-792 Return only experiments and projects that a user has access to after logging in (#122)

* Only list projects with access

* Check for missing user.

No exception on empty list

* Fixed query

* Accpet empty arrays

* Fix length check

* Restore change lost on merge

* Update tests

* Fix transition between gem2s and data processing (#130)

* Fix discrepancies in qc and pipeline renaming

* Fix various bugs

* When no samples, change 500 for 404

* Implement xaviers fix on statusResToSend

* Change updateExperiment to work by doing one single request

* Refactor changes in previous commit

* Fix qc/pipeline discrepancy problem on work requests send

* Change names to more readable options

* Rename helpers

* Minor rafctoring based on agi comment

Co-authored-by: Martin Fosco <[email protected]>
Co-authored-by: Xavier Verges <[email protected]>
Co-authored-by: Martin Fosco <[email protected]>

* Handle gem2s response refactoring (#129)

* Adapt response swagger validation

* Add missing change from status to taskName

Co-authored-by: Martin Fosco <[email protected]>

* Hide some wrong xray missing context logs + typos (#133)

* fix nonexistent property access (#137)

* [BIOMAGE-1037] - Pass metadata to gem2s (#135)

* pass metadata to gem2s
* exclude metadata if empty

* [BIOMAGE-1036] - Run QC after GEM2S (#134)

* run qc after gem2s
* add test
* await for pipeline runs

* [BIOMAGE-1009] Fix error sns messages (#136)

* Adapt code in api validation and handler in gem2s to new structure

* Fix response handlers

* Minor fixes and refactoring

* Update test snapshots

* Fix indentation

* Fix validation swagger files

* Fix tests

Co-authored-by: Martin Fosco <[email protected]>
Co-authored-by: Martin Fosco <[email protected]>

* convert to async/await (#141)

* Beter 404 for plots-tables (#140)

* Beter 404 for plots-tables

* Friendlier logging of expected errors

* Broken if omg

* Better logging

* Log hook execution

* Wrong condition, again :-(

* Take care of falsy parsedMessage

* Sanitize metadata key (#142)

* [BIOMAGE-1027] Add safeBatchGetItem that divides batchGetItem in groups of requests of 100 keys (#138)

* Add safeBatchGetItem

* Add tests

* Add explanatory comments

* Rewrite safeBatchGetItem to work with limit of 100 for total of items instead of per table

* Minor bugfix and add more unit tests

* Remove obsolete object

Co-authored-by: Martin Fosco <[email protected]>

* [BIOMAGE-989] - Work request throw error if pipeline has not yet succeeded (#139)

* new flow for releases (#143)

* added deployment for release/hotfixes PRs to master (#145)

* using tokens to allow flow to trigger testing

* building releases/hotfixes on PR instead of push

* Update ci-develop.yaml

* Delete files in s3 for deleted samples (#146)

* Add deletion of files of deleted project

* Add basic delete single sample operation (still needs updates)

* Remove samples unnecessary ids field

* Fixes and add validation

* Fix tests

* Fix tests and cleanup code

* Reorder

* Get sample files instead of hardcoded files

* Fix tests

Co-authored-by: Martin Fosco <[email protected]>

* notifying worker of job done (#151)

* notifying worker of job done

* Fix louvain cellsets storage in s3

* Add small comment

* clean up

Co-authored-by: Martin Fosco <[email protected]>

* [BIOMAGE-1044] Add a needsRunning flag to gem2s pipeline status (#147)

* Add needsRunning flag to gem2s status

Adds a paramsHash to the gem2s handle

* Minor readibility improvements and cleanup

* Refactoring and fix issue with taskparams hash

* Refactoring and fix tests

* Fix errro during run

* Refactor fix

Co-authored-by: Martin Fosco <[email protected]>
Co-authored-by: Martin Fosco <[email protected]>
Co-authored-by: Pol Alvarez <[email protected]>

* Deprecated change for ci.yaml

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Anugerah Erlaut <[email protected]>
Co-authored-by: Juanlu <[email protected]>
Co-authored-by: Xavier Vergés <[email protected]>
Co-authored-by: ivababukova <[email protected]>
Co-authored-by: Martin Fosco <[email protected]>
Co-authored-by: Martin Fosco <[email protected]>
Co-authored-by: Marcell Pék <[email protected]>
Co-authored-by: Marcell Pek <[email protected]>
Co-authored-by: StefanBabukov <[email protected]>
Co-authored-by: Alex Pickering <[email protected]>
Co-authored-by: Martin Fosco <[email protected]>
Co-authored-by: Martin Fosco <[email protected]>
alexvpickering pushed a commit that referenced this pull request Mar 14, 2023
[BIOMAGE-2328] - Add sandbox id to batch jobdef
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants