Skip to content

Commit

Permalink
fix(ids-admin|user-profile): Apply 15968 and 15990 to current release (
Browse files Browse the repository at this point in the history
…#16000)

* fix(services-auth-admin-api): Fix delegation settings (#15968)

* Fix admin api scope added delegation type update to only update requested type.

* chore: nx format:write update dirty files

---------

Co-authored-by: andes-it <[email protected]>

* fix(user-profile): Check if nationalId+deviceToken pair exists and only create if not. (#15990)

* Use upsert instead of create to stop unique constraint errors

* Upsert is not converting properties to propper db column names with underscore. So moving to a simple findOne and create if not found.

* Fix addDeviceToken test

---------

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

---------

Co-authored-by: andes-it <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
  • Loading branch information
3 people authored Sep 13, 2024
1 parent a6a61ab commit 431712f
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
TranslatedValueDto,
ApiScopeDelegationType,
AdminPatchScopeDto,
ApiScope,
} from '@island.is/auth-api-lib'
import { FixtureFactory } from '@island.is/services/auth/testing'
import {
Expand Down Expand Up @@ -763,6 +764,7 @@ describe('MeScopesController', () => {
let app: TestApp
let server: request.SuperTest<request.Test>
let apiScopeDelegationTypeModel: typeof ApiScopeDelegationType
let fixtureFactory: FixtureFactory

beforeAll(async () => {
app = await setupApp({
Expand All @@ -772,6 +774,7 @@ describe('MeScopesController', () => {
dbType: 'postgres',
})
server = request(app.getHttpServer())
fixtureFactory = new FixtureFactory(app)

apiScopeDelegationTypeModel = await app.get(
getModelToken(ApiScopeDelegationType),
Expand Down Expand Up @@ -886,6 +889,59 @@ describe('MeScopesController', () => {
},
})
})

it('should only update requested delegation setting fields', async () => {
// Arrange
// Create new subject under testing test data to control initial state of delegation settings.
const sutScope = await fixtureFactory.createApiScope({
domainName: TENANT_ID,
allowExplicitDelegationGrant: true,
supportedDelegationTypes: [AuthDelegationType.Custom],
})

// Act - Update partially delegation setting
const response = await server
.patch(
`/v2/me/tenants/${TENANT_ID}/scopes/${encodeURIComponent(
sutScope.name,
)}`,
)
.send({
addedDelegationTypes: [AuthDelegationType.ProcurationHolder],
})

// Assert that we only updated requested delegation setting fields
expect(response.status).toEqual(200)
expect(response.body).toMatchObject({
...sutScope.toDTO(),
displayName: [
{
locale: 'is',
value: sutScope.displayName,
},
],
description: [
{
locale: 'is',
value: sutScope.description,
},
],
grantToProcuringHolders: true,
supportedDelegationTypes: expect.arrayContaining([
AuthDelegationType.Custom,
AuthDelegationType.ProcurationHolder,
]),
} as AdminScopeDTO)
const apiScopeDelegationTypes = await apiScopeDelegationTypeModel.findAll(
{
where: {
apiScopeName: sutScope.name,
},
},
)

expect(apiScopeDelegationTypes).toHaveLength(2)
})
})

describe('POST: /v2/me/tenants/:tenantId/scopes', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -816,21 +816,23 @@ describe('User profile API', () => {
)
})

it('POST /userProfile/{nationalId}/device-tokens duplicate token should return 400 bad request', async () => {
it('POST /userProfile/{nationalId}/device-tokens duplicate token should return the existing token', async () => {
// create it
await request(app.getHttpServer())
const res1 = await request(app.getHttpServer())
.post(`/userProfile/${mockProfile.nationalId}/device-tokens`)
.send({
deviceToken: mockDeviceToken.deviceToken,
})
.expect(201)
// try to create same again
await request(app.getHttpServer())
const res2 = await request(app.getHttpServer())
.post(`/userProfile/${mockProfile.nationalId}/device-tokens`)
.send({
deviceToken: mockDeviceToken.deviceToken,
})
.expect(400)
.expect(201)

expect(res1.body).toEqual(res2.body)
})

it('POST /userProfile/{nationalId}/device-tokens with missing payload should 400 bad request', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,10 @@ export class UserProfileController {
} else {
// findOrCreateUserProfile for edge cases - fragmented onboarding
await this.findOrCreateUserProfile(nationalId, user)
return await this.userProfileService.addDeviceToken(body, user)
// The behaviour of returning the token if it already exists is not following API Design Guide
// It should respond with 303 See Other and a Location header to the existing resource
// But as the v1 of the user profile is not following this, we will keep the same behaviour.
return this.userProfileService.addDeviceToken(body, user)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,14 @@ export class UserProfileService {

async addDeviceToken(body: DeviceTokenDto, user: User) {
try {
const token = await this.userDeviceTokensModel.findOne({
where: { nationalId: user.nationalId, deviceToken: body.deviceToken },
})

if (token) {
return token
}

return await this.userDeviceTokensModel.create({
...body,
nationalId: user.nationalId,
Expand Down
12 changes: 8 additions & 4 deletions libs/auth-api-lib/src/lib/resources/admin/admin-scope.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -480,10 +480,14 @@ export class AdminScopeService {
) {
await this.apiScope.update(
{
grantToLegalGuardians,
grantToPersonalRepresentatives,
grantToProcuringHolders,
allowExplicitDelegationGrant,
...(grantToLegalGuardians ? { grantToLegalGuardians } : {}),
...(grantToPersonalRepresentatives
? { grantToPersonalRepresentatives }
: {}),
...(grantToProcuringHolders ? { grantToProcuringHolders } : {}),
...(allowExplicitDelegationGrant
? { allowExplicitDelegationGrant }
: {}),
},
{
transaction,
Expand Down

0 comments on commit 431712f

Please sign in to comment.