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

Fix/restrict privatisation to email login #840

Conversation

alexanderleegs
Copy link
Contributor

This PR adds an additional restriction to the repo privatisation feature such that only email login users can see the feature - this is due to the prerequisite of restricting github access before being able to convert repos to private, as private repos are unable to set additional branch protection rules, meaning the PR approval requirement can be bypassed. To be reviewed in conjunction with PR #1344 on the isomercms-frontend repo.

Copy link
Contributor

@kishore03109 kishore03109 left a comment

Choose a reason for hiding this comment

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

while the code changes lgtm, I am slightly confused about how this is called in the FE and if we are parsing the response of a string OK correctly when the user is not an email based login. Happy to hop onto a huddle to unblock you on this

@@ -129,10 +130,12 @@ class SettingsRouter {
router.post("/", attachRollbackRouteHandlerWrapper(this.updateSettingsPage))
router.get(
"/repo-password",
Copy link
Contributor

Choose a reason for hiding this comment

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

Extremely basic qn: where exactly in the FE are we calling this ah? I couldnt find any corresponding one, the only enpoint that the corresponding FE pr seems to call is /sites/<siteName>/settings, is this intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The corresponding FE PR is only to restrict the privatisation route on the FE to email login as well! the call site is located in the base PR, isomerpages/isomercms-frontend#1316

@alexanderleegs alexanderleegs merged commit 83243e0 into feat/privatise-netlify-sites Jul 19, 2023
@mergify mergify bot deleted the fix/restrict-privatisation-to-email-login branch July 19, 2023 04:51
alexanderleegs added a commit that referenced this pull request Jul 19, 2023
* chore: add new env var for netlify access token

* feat: add netlify privatisation

* chore: update .env.test

* chore: add return type

* chore: updates stop_builds type

* chore: cast error

* chore: move netlify conditional out separately

* fix: check for error type

* chore: shift and rename file

* Fix: refactor axios-utils and add isAxiosError

* refactor: pass only password to updateNetlifySite

* fix: use isAxiosError

* fix: revert to require

* chore: swap to @utils

* chore: modify method name

* chore: rename netlifyService to netlifyApi

* Fix/restrict privatisation to email login (#840)

* fix: add verifyIsEmailUser wrapper to password endpoints

* chore: update tests
alexanderleegs added a commit that referenced this pull request Jul 19, 2023
* chore: add env var and reformat env-example

* feat: modify db and add migrations

* feat: add util methods for password decryption

* feat: add changeRepoPrivacy

* feat: add deploymentClient updateBranch methods

* feat: add deploymentService methods

* feat: add methods to get and retrieve password

* feat: add get and update password endpoints

* chore: add schema

* chore: update initialisation

* chore: update db schema to include date of password

* feat: add functionality to remove password

* fix: change api to send/receive password directly

* chore: swap to kebab-case

* chore: underscore unused vars

* chore: fix name

* chore: update comment

* chore: update wrap

* chore: separate update and delete password

* chore: split delete path into separate method

* fix: set secret key to required

* chore: rename arg

* Fix: handle case where no password change necessary

* chore: enforce that decrypted password is not modified

* chore: update error type

* fix: revert casting of amplify methods

* chore: pass key as argument

* chore: update DTO for netlify sites

* Test/repo privatisation (#807)

* chore: update site fixtures

* test: add githubService tests

* test: add settings router test

* Fix: settings tests

* chore: update env vars for tests

* chore: update fixtures

* fix: requestSchema

* fix: use writehandler instead of rollback handler

* chore: update tests to use new dto

* feat: add crypto-utils test

* feat: add integration test

* fix: update updatePassword schema

* fix: update crypto-utils tests

* chore: remove unused imports

* chore: update names of imported fixtures

* feat: add new test case for invalid req

* chore: update const name

* fix: test input for new dto

* chore: add validator

* chore: update tests

* nit: update test names

* nit: rename repository to deploymentsRepository

* fix: var name

* Feat/privatise netlify sites (#828)

* chore: add new env var for netlify access token

* feat: add netlify privatisation

* chore: update .env.test

* chore: add return type

* chore: updates stop_builds type

* chore: cast error

* chore: move netlify conditional out separately

* fix: check for error type

* chore: shift and rename file

* Fix: refactor axios-utils and add isAxiosError

* refactor: pass only password to updateNetlifySite

* fix: use isAxiosError

* fix: revert to require

* chore: swap to @utils

* chore: modify method name

* chore: rename netlifyService to netlifyApi

* Fix/restrict privatisation to email login (#840)

* fix: add verifyIsEmailUser wrapper to password endpoints

* chore: update tests
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