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.

Sign up for GitHub

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

Jump to bottom

feat(auth-admin): Webhook for general mandate delegation #16257

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+377 −58
Open

feat(auth-admin): Webhook for general mandate delegation #16257

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
22fa524
gm-delegation-webhook
GunnlaugurG Oct 3, 2024
3ed8368
chore: charts update dirty files
andes-it Oct 3, 2024
eeb80b0
pr comments fixes
GunnlaugurG Oct 4, 2024
3f5870f
pr comments fixes
GunnlaugurG Oct 4, 2024
3671462
fix broken test
GunnlaugurG Oct 4, 2024
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
2 changes: 2 additions & 0 deletions apps/services/auth/admin-api/infra/auth-admin-api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ export const serviceSetup = (): ServiceBuilder<'services-auth-admin-api'> => {
.secrets({
ZENDESK_CONTACT_FORM_EMAIL: '/k8s/api/ZENDESK_CONTACT_FORM_EMAIL',
ZENDESK_CONTACT_FORM_TOKEN: '/k8s/api/ZENDESK_CONTACT_FORM_TOKEN',
ZENDESK_WEBHOOK_SECRET_GENERAL_MANDATE:
'/k8s/services-auth/ZENDESK_WEBHOOK_SECRET_GENERAL_MANDATE',
CLIENT_SECRET_ENCRYPTION_KEY:
'/k8s/services-auth/admin-api/CLIENT_SECRET_ENCRYPTION_KEY',
IDENTITY_SERVER_CLIENT_SECRET:
Expand Down
28 changes: 26 additions & 2 deletions apps/services/auth/admin-api/src/app/v2/delegations/delegation-admin.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,30 +9,41 @@ import {
UseGuards,
} from '@nestjs/common'
import { ApiTags } from '@nestjs/swagger'
import flatMap from 'lodash/flatMap'

import {
BypassAuth,
CurrentUser,
IdsUserGuard,
Scopes,
ScopesGuard,
User,
ZendeskAuthGuard,
} from '@island.is/auth-nest-tools'
import {
CreatePaperDelegationDto,
DelegationAdminCustomDto,
DelegationAdminCustomService,
DelegationDTO,
ZendeskWebhookInputDto,
} from '@island.is/auth-api-lib'
import { Documentation } from '@island.is/nest/swagger'
import { Audit, AuditService } from '@island.is/nest/audit'
import { DelegationAdminScopes } from '@island.is/auth/scopes'
import flatMap from 'lodash/flatMap'
import { isDefined } from '@island.is/shared/utils'

const namespace = '@island.is/auth/delegation-admin'

const ZENDESK_WEBHOOK_SECRET_GENERAL_MANDATE =
process.env.ZENDESK_WEBHOOK_SECRET_GENERAL_MANDATE

if (!ZENDESK_WEBHOOK_SECRET_GENERAL_MANDATE) {
throw new Error(
'Environment variable ZENDESK_WEBHOOK_SECRET_GENERAL_MANDATE must be set',
)
}

@UseGuards(IdsUserGuard, ScopesGuard)
@Scopes(DelegationAdminScopes.read)
GunnlaugurG marked this conversation as resolved.
Show resolved Hide resolved
@ApiTags('delegation-admin')
@Controller('delegation-admin')
@Audit({ namespace })
Expand All @@ -43,6 +54,7 @@ export class DelegationAdminController {
) {}

@Get()
@Scopes(DelegationAdminScopes.read)
@Documentation({
response: { status: 200, type: DelegationAdminCustomDto },
request: {
Expand Down Expand Up @@ -91,6 +103,18 @@ export class DelegationAdminController {
)
}

@BypassAuth()
@UseGuards(ZendeskAuthGuard(ZENDESK_WEBHOOK_SECRET_GENERAL_MANDATE))
@Post('/zendesk')
@Documentation({
response: { status: 200 },
})
async createByZendeskId(
@Body() { id }: ZendeskWebhookInputDto,
): Promise<void> {
await this.delegationAdminService.createDelegationByZendeskId(id)
}

@Delete(':delegationId')
@Scopes(DelegationAdminScopes.admin)
@Documentation({
Expand Down
80 changes: 79 additions & 1 deletion apps/services/auth/admin-api/src/app/v2/delegations/test/delegation-admin.auth.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import request from 'supertest'
import bodyParser from 'body-parser'

import {
getRequestMethod,
Expand All @@ -11,9 +12,11 @@ import { User } from '@island.is/auth-nest-tools'
import { FixtureFactory } from '@island.is/services/auth/testing'
import { createCurrentUser } from '@island.is/testing/fixtures'
import { DelegationAdminScopes } from '@island.is/auth/scopes'
import { SequelizeConfigService } from '@island.is/auth-api-lib'
import { DelegationDTO, SequelizeConfigService } from '@island.is/auth-api-lib'
import { DelegationAdminCustomService } from '@island.is/auth-api-lib'

import { AppModule } from '../../../app.module'
import { includeRawBodyMiddleware } from '@island.is/infra-nest-server'

describe('withoutAuth and permissions', () => {
async function formatUrl(app: TestApp, endpoint: string, user?: User) {
Expand Down Expand Up @@ -132,4 +135,79 @@ describe('withoutAuth and permissions', () => {
app.cleanUp()
},
)

describe('POST /delegation-admin/:zendeskId', () => {
let app: TestApp
let server: request.SuperTest<request.Test>
let delegationAdminService: DelegationAdminCustomService

beforeEach(async () => {
app = await setupAppWithoutAuth({
AppModule,
SequelizeConfigService,
dbType: 'postgres',
beforeServerStart: async (app) => {
await new Promise((resolve) =>
resolve(app.use(includeRawBodyMiddleware())),
)
},
})
Comment on lines +149 to +154
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Simplify the beforeServerStart function.

The beforeServerStart function can be simplified. The current implementation wraps the synchronous app.use call in an unnecessary Promise. Consider refactoring it as follows:

beforeServerStart: async (app) => {
  app.use(includeRawBodyMiddleware())
},

This change maintains the same functionality while making the code more straightforward and easier to read.


server = request(app.getHttpServer())

delegationAdminService = app.get(DelegationAdminCustomService)

jest
.spyOn(delegationAdminService, 'createDelegationByZendeskId')
.mockImplementation(() => Promise.resolve({} as DelegationDTO))
})

afterEach(() => {
app.cleanUp()
})

it('POST /delegation-admin/zendesk should return 403 Forbidden when request signature is invalid.', async () => {
// Act
const res = await getRequestMethod(
server,
'POST',
)('/delegation-admin/zendesk')
.send({
id: 'Incorrect body',
})
.set(
'x-zendesk-webhook-signature',
'6sUtGV8C8OdoGgCdsV2xRm3XeskZ33Bc5124RiAK4Q4=',
)
.set('x-zendesk-webhook-signature-timestamp', '2024-10-02T14:21:04Z')

// Assert
expect(res.status).toEqual(403)
expect(res.body).toMatchObject({
status: 403,
type: 'https://httpstatuses.org/403',
title: 'Forbidden',
detail: 'Forbidden resource',
})
})

it('POST /delegation-admin/zendesk should return 201 when signature is valid', async () => {
// Act
const res = await getRequestMethod(
server,
'POST',
)('/delegation-admin/zendesk')
.send({
id: 'test',
})
.set(
'x-zendesk-webhook-signature',
'ntgS06VGgd4z73lHjIpC2sk9azhRNi4u1xkXF/KPKTs=',
)
.set('x-zendesk-webhook-signature-timestamp', '2024-10-02T14:21:04Z')

// Assert
expect(res.status).toEqual(200)
})
Comment on lines +194 to +211
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance test assertions for valid signature scenario.

While the test case correctly checks for a 200 OK response when the signature is valid, it could be improved:

  1. Add an assertion to verify that delegationAdminService.createDelegationByZendeskId was called with the correct parameters.
  2. Include an assertion for the response body structure.

Consider enhancing the test with these additional assertions:

// Assert
expect(res.status).toEqual(200)
expect(delegationAdminService.createDelegationByZendeskId).toHaveBeenCalledWith('test')
expect(res.body).toEqual({}) // Adjust this based on the expected response structure

These additions will make the test more robust by ensuring that the service method is called correctly and that the response body matches the expected structure.

})
})
35 changes: 33 additions & 2 deletions apps/services/auth/admin-api/src/app/v2/delegations/test/delegation-admin.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,15 +224,25 @@ describe('DelegationAdmin - With authentication', () => {
const mockZendeskService = (
toNationalId: string,
fromNationalId: string,
info?: {
tags?: string[]
status?: TicketStatus
},
) => {
const { tags, status } = {
tags: [DELEGATION_TAG],
status: TicketStatus.Solved,
...info,
}

zendeskServiceApiSpy = jest
.spyOn(zendeskService, 'getTicket')
.mockImplementation((ticketId: string) => {
return new Promise((resolve) =>
resolve({
id: ticketId,
tags: [DELEGATION_TAG],
status: TicketStatus.Solved,
tags: tags,
status: status,
custom_fields: [
{
id: ZENDESK_CUSTOM_FIELDS.DelegationToReferenceId,
Expand Down Expand Up @@ -328,5 +338,26 @@ describe('DelegationAdmin - With authentication', () => {
// Assert
expect(res.status).toEqual(400)
})

it('POST /delegation-admin should not create delegation with incorrect zendesk ticket status', async () => {
// Arrange
mockZendeskService(toNationalId, fromNationalId, {
status: TicketStatus.Open,
})

const delegation: CreatePaperDelegationDto = {
toNationalId,
fromNationalId,
referenceId: 'ref1',
}

// Act
const res = await getRequestMethod(
server,
'POST',
)('/delegation-admin').send(delegation)

expect(res.status).toEqual(400)
})
})
})
8 changes: 7 additions & 1 deletion apps/services/auth/admin-api/src/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { bootstrap } from '@island.is/infra-nest-server'
import {
bootstrap,
includeRawBodyMiddleware,
} from '@island.is/infra-nest-server'

import { AppModule } from './app/app.module'
import { environment as env } from './environments'
Expand All @@ -14,4 +17,7 @@ bootstrap({
healthCheck: {
database: true,
},
beforeServerStart: async (app) => {
app.use(includeRawBodyMiddleware())
GunnlaugurG marked this conversation as resolved.
Show resolved Hide resolved
},
})
1 change: 1 addition & 0 deletions charts/identity-server/values.dev.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ services-auth-admin-api:
SYSLUMENN_USERNAME: '/k8s/services-auth/SYSLUMENN_USERNAME'
ZENDESK_CONTACT_FORM_EMAIL: '/k8s/api/ZENDESK_CONTACT_FORM_EMAIL'
ZENDESK_CONTACT_FORM_TOKEN: '/k8s/api/ZENDESK_CONTACT_FORM_TOKEN'
ZENDESK_WEBHOOK_SECRET_GENERAL_MANDATE: '/k8s/services-auth/ZENDESK_WEBHOOK_SECRET_GENERAL_MANDATE'
securityContext:
allowPrivilegeEscalation: false
privileged: false
Expand Down
1 change: 1 addition & 0 deletions charts/identity-server/values.prod.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ services-auth-admin-api:
SYSLUMENN_USERNAME: '/k8s/services-auth/SYSLUMENN_USERNAME'
ZENDESK_CONTACT_FORM_EMAIL: '/k8s/api/ZENDESK_CONTACT_FORM_EMAIL'
ZENDESK_CONTACT_FORM_TOKEN: '/k8s/api/ZENDESK_CONTACT_FORM_TOKEN'
ZENDESK_WEBHOOK_SECRET_GENERAL_MANDATE: '/k8s/services-auth/ZENDESK_WEBHOOK_SECRET_GENERAL_MANDATE'
securityContext:
allowPrivilegeEscalation: false
privileged: false
Expand Down
1 change: 1 addition & 0 deletions charts/identity-server/values.staging.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ services-auth-admin-api:
SYSLUMENN_USERNAME: '/k8s/services-auth/SYSLUMENN_USERNAME'
ZENDESK_CONTACT_FORM_EMAIL: '/k8s/api/ZENDESK_CONTACT_FORM_EMAIL'
ZENDESK_CONTACT_FORM_TOKEN: '/k8s/api/ZENDESK_CONTACT_FORM_TOKEN'
ZENDESK_WEBHOOK_SECRET_GENERAL_MANDATE: '/k8s/services-auth/ZENDESK_WEBHOOK_SECRET_GENERAL_MANDATE'
securityContext:
allowPrivilegeEscalation: false
privileged: false
Expand Down
1 change: 1 addition & 0 deletions libs/auth-api-lib/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export * from './lib/delegations/dto/delegation-index.dto'
export * from './lib/delegations/dto/paginated-delegation-provider.dto'
export * from './lib/delegations/dto/delegation-provider.dto'
export * from './lib/delegations/dto/merged-delegation.dto'
export * from './lib/delegations/dto/zendesk-webhook-input.dto'
export * from './lib/delegations/models/delegation.model'
export * from './lib/delegations/models/delegation.model'
export * from './lib/delegations/models/delegation-scope.model'
Expand Down
Loading
Loading