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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
29 changes: 27 additions & 2 deletions src/integration/Privatisation.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
Site,
SiteMember,
User,
Whitelist,
} from "@database/models"
import { generateRouterForUserWithSite } from "@fixtures/app"
import {
Expand All @@ -39,6 +40,7 @@ import {
MOCK_USER_DBENTRY_THREE,
MOCK_USER_DBENTRY_TWO,
} from "@fixtures/users"
import { AuthorizationMiddleware } from "@root/middleware/authorization"
import { SettingsRouter as _SettingsRouter } from "@root/routes/v2/authenticatedSites/settings"
import { SettingsService } from "@root/services/configServices/SettingsService"
import { BaseDirectoryService } from "@root/services/directoryServices/BaseDirectoryService"
Expand All @@ -54,11 +56,13 @@ import { CollectionYmlService } from "@root/services/fileServices/YmlFileService
import { ConfigService } from "@root/services/fileServices/YmlFileServices/ConfigService"
import { FooterYmlService } from "@root/services/fileServices/YmlFileServices/FooterYmlService"
import { NavYmlService } from "@root/services/fileServices/YmlFileServices/NavYmlService"
import CollaboratorsService from "@root/services/identity/CollaboratorsService"
import DeploymentsService from "@root/services/identity/DeploymentsService"
import AuthorizationMiddlewareService from "@root/services/middlewareServices/AuthorizationMiddlewareService"
import { GitHubService } from "@services/db/GitHubService"
import * as ReviewApi from "@services/db/review"
import { ConfigYmlService } from "@services/fileServices/YmlFileServices/ConfigYmlService"
import { getUsersService } from "@services/identity"
import { getIdentityAuthService, getUsersService } from "@services/identity"
import IsomerAdminsService from "@services/identity/IsomerAdminsService"
import SitesService from "@services/identity/SitesService"
import ReviewRequestService from "@services/review/ReviewRequestService"
Expand Down Expand Up @@ -155,6 +159,24 @@ const deploymentsService = new DeploymentsService({
deploymentsRepository: Deployment,
})

const identityAuthService = getIdentityAuthService(gitHubService)
const collaboratorsService = new CollaboratorsService({
siteRepository: Site,
siteMemberRepository: SiteMember,
sitesService,
usersService,
whitelist: Whitelist,
})
const authorizationMiddlewareService = new AuthorizationMiddlewareService({
identityAuthService,
usersService,
isomerAdminsService,
collaboratorsService,
})
const authorizationMiddleware = new AuthorizationMiddleware({
authorizationMiddlewareService,
})

const settingsService = new SettingsService({
configYmlService,
footerYmlService,
Expand All @@ -164,7 +186,10 @@ const settingsService = new SettingsService({
deploymentsService,
gitHubService,
})
const SettingsRouter = new _SettingsRouter({ settingsService })
const SettingsRouter = new _SettingsRouter({
settingsService,
authorizationMiddleware,
})
const settingsSubrouter = SettingsRouter.getRouter()
const subrouter = express()
subrouter.use("/:siteName", settingsSubrouter)
Expand Down
3 changes: 3 additions & 0 deletions src/routes/v2/authenticatedSites/__tests__/Settings.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,11 @@ describe("Settings Router", () => {
extractNavFields: SettingsService.extractNavFields,
}

const mockAuthorizationMiddleware = jest.fn()

const router = new SettingsRouter({
settingsService: mockSettingsService,
authorizationMiddleware: mockAuthorizationMiddleware,
})

const subrouter = express()
Expand Down
5 changes: 4 additions & 1 deletion src/routes/v2/authenticatedSites/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,10 @@ const getAuthenticatedSitesSubrouter = ({
})
const contactUsV2Router = new ContactUsRouter({ contactUsPageService })
const homepageV2Router = new HomepageRouter({ homepagePageService })
const settingsV2Router = new SettingsRouter({ settingsService })
const settingsV2Router = new SettingsRouter({
settingsService,
authorizationMiddleware,
})
const navigationV2Router = new NavigationRouter({
navigationYmlService: navYmlService,
})
Expand Down
5 changes: 4 additions & 1 deletion src/routes/v2/authenticatedSites/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ const { isPasswordValid } = require("@root/validators/validators")
const { SettingsService } = require("@services/configServices/SettingsService")

class SettingsRouter {
constructor({ settingsService }) {
constructor({ settingsService, authorizationMiddleware }) {
this.authorizationMiddleware = authorizationMiddleware
this.settingsService = settingsService
// We need to bind all methods because we don't invoke them from the class directly
autoBind(this)
Expand Down Expand Up @@ -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

this.authorizationMiddleware.verifyIsEmailUser,
attachReadRouteHandlerWrapper(this.getRepoPassword)
)
router.post(
"/repo-password",
this.authorizationMiddleware.verifyIsEmailUser,
attachWriteRouteHandlerWrapper(this.updateRepoPassword)
)

Expand Down