Skip to content

Commit

Permalink
Merge branch 'main' into j-s/display-unserviced-tag
Browse files Browse the repository at this point in the history
  • Loading branch information
unakb authored Dec 23, 2024
2 parents d8cc078 + 56493cc commit d97d7ab
Show file tree
Hide file tree
Showing 13 changed files with 325 additions and 74 deletions.
65 changes: 52 additions & 13 deletions apps/services/bff/src/app/modules/auth/auth.controller.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ConfigType } from '@island.is/nest/config'
import { LOGIN_ATTEMPT_FAILED_ACTIVE_SESSION } from '@island.is/shared/constants'
import { CACHE_MANAGER } from '@nestjs/cache-manager'
import { HttpStatus, INestApplication } from '@nestjs/common'
import jwt from 'jsonwebtoken'
Expand Down Expand Up @@ -115,9 +116,8 @@ describe('AuthController', () => {

expect(key).toEqual(`attempt::${mockConfig.name}::${SID_VALUE}`)
expect(value).toMatchObject({
originUrl: baseUrlWithKey,
codeVerifier: expect.any(String),
targetLinkUri: allowedTargetLinkUri,
targetLinkUri: allowedTargetLinkUri ?? baseUrlWithKey,
})
})

Expand Down Expand Up @@ -169,8 +169,8 @@ describe('AuthController', () => {
const invalidTargetLinkUri = 'http://test-client.com/invalid'

const searchParams = new URLSearchParams({
bff_error_code: '400',
bff_error_description: 'Login failed!',
bff_error_status_code: '400',
bff_error_message: 'Login failed!',
})

const errorUrl = `${baseUrlWithKey}?${searchParams.toString()}`
Expand Down Expand Up @@ -246,8 +246,8 @@ describe('AuthController', () => {
// Arrange
const idsError = 'Invalid request'
const searchParams = new URLSearchParams({
bff_error_code: '500',
bff_error_description: idsError,
bff_error_status_code: '500',
bff_error_message: idsError,
})

const errorUrl = `${baseUrlWithKey}?${searchParams.toString()}`
Expand All @@ -265,8 +265,8 @@ describe('AuthController', () => {
it('should validate query string params and redirect with error if invalid', async () => {
// Arrange
const searchParams = new URLSearchParams({
bff_error_code: '400',
bff_error_description: 'Login failed!',
bff_error_status_code: '400',
bff_error_message: 'Login failed!',
})
const errorUrl = `${baseUrlWithKey}?${searchParams.toString()}`

Expand All @@ -281,7 +281,7 @@ describe('AuthController', () => {
const scenarios = [
{
description:
'should successfully finish callback login and redirect to fallback originUrl',
'should successfully finish callback login and redirect to fallback targetLinkUri',
targetLinkUri: undefined,
expectedLocation: 'http://test-client.com/testclient',
},
Expand Down Expand Up @@ -315,9 +315,8 @@ describe('AuthController', () => {
`attempt::${mockConfig.name}::${SID_VALUE}`,
)
expect(loginAttempt[1]).toMatchObject({
originUrl: baseUrlWithKey,
codeVerifier: expect.any(String),
targetLinkUri,
targetLinkUri: targetLinkUri ?? baseUrlWithKey,
})

// Then make a callback to the login endpoint
Expand Down Expand Up @@ -378,8 +377,8 @@ describe('AuthController', () => {
it('should redirect to logout redirect uri with error params if cookie and session state do not match', async () => {
// Arrange
const searchParams = new URLSearchParams({
bff_error_code: '400',
bff_error_description: 'Logout failed!',
bff_error_status_code: '400',
bff_error_message: 'Logout failed!',
})

const errorUrl = `${allowedTargetLinkUri}?${searchParams.toString()}`
Expand Down Expand Up @@ -531,5 +530,45 @@ describe('AuthController', () => {
message: 'Logout successful!',
})
})

it('should detect double session when attempt data exists in current session', async () => {
// Arrange
const code = 'testcode'
const getCacheSpy = jest.spyOn(mockCacheManagerValue, 'get')
const searchParams = new URLSearchParams({
bff_error_status_code: '409',
bff_error_message: 'Multiple sessions detected!',
bff_error_code: LOGIN_ATTEMPT_FAILED_ACTIVE_SESSION,
})
const errorUrl = `${baseUrlWithKey}?${searchParams.toString()}`

// First login - create initial session with attempt data
await server.get('/login')

const currentKey = `current::${mockConfig.name}::${SID_VALUE}`

mockCacheManagerValue.set(currentKey, tokensResponse)

getCacheSpy.mockImplementation((key) => {
if (key === currentKey) {
return Promise.resolve(tokensResponse)
}

return Promise.resolve(null)
})

// Second login attempt (user pressed back)
await server.get('/login')

// Act - Try to complete second login
const res = await server
.get('/callbacks/login')
.set('Cookie', [`${SESSION_COOKIE_NAME}=${SID_VALUE}`])
.query({ code, state: SID_VALUE })

// Assert
expect(res.status).toEqual(HttpStatus.FOUND)
expect(res.headers.location).toEqual(errorUrl)
})
})
})
88 changes: 65 additions & 23 deletions apps/services/bff/src/app/modules/auth/auth.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type { Request, Response } from 'express'
import jwksClient from 'jwks-rsa'
import { jwtDecode } from 'jwt-decode'

import { LOGIN_ATTEMPT_FAILED_ACTIVE_SESSION } from '@island.is/shared/constants'
import { IdTokenClaims } from '@island.is/shared/types'
import { Algorithm, decode, verify } from 'jsonwebtoken'
import { v4 as uuid } from 'uuid'
Expand Down Expand Up @@ -86,11 +87,14 @@ export class AuthService {
res: Response,
args?: Partial<CreateErrorQueryStrArgs> & { targetUrl?: string },
) {
const code = args?.code || 500
const error = args?.error || 'Login failed!'
const statusCode = args?.statusCode || 500
const message = args?.message || 'Login failed!'

return res.redirect(
this.createClientBaseUrlWithError({ code, error }, args?.targetUrl),
this.createClientBaseUrlWithError(
{ code: args?.code, message, statusCode },
args?.targetUrl,
),
)
}

Expand Down Expand Up @@ -171,7 +175,7 @@ export class AuthService {
)

return this.redirectWithError(res, {
code: 400,
statusCode: 400,
})
}

Expand All @@ -189,11 +193,12 @@ export class AuthService {
await this.cacheService.save({
key: this.cacheService.createSessionKeyType('attempt', attemptLoginId),
value: {
// Fallback if targetLinkUri is not provided
originUrl: this.createClientBaseUrl(),
// Code verifier to be used in the callback
codeVerifier,
targetLinkUri,
targetLinkUri:
targetLinkUri ??
// Fallback if targetLinkUri is not provided
this.createClientBaseUrl(),
},
ttl: this.config.cacheLoginAttemptTTLms,
})
Expand Down Expand Up @@ -235,6 +240,45 @@ export class AuthService {
}
}

/**
* Handles cases where a login attempt cache entry is not found during the callback phase.
* This typically occurs in one of these scenarios:
*
* 1. The login attempt cache has expired (TTL exceeded).
* 2. The cache entry was deleted.
* 3. The user attempted to reuse a callback URL after a successful login
* (e.g., by using browser back button after logging in)
*
* Recovery process:
* 1. Checks if there's an existing active session (via session cookie)
* 2. If a session exists, returns a 409 Conflict indicating multiple session attempt
* 3. If no recovery is possible, redirects to error page and log as warning
*/
private async handleMissingLoginAttempt({
req,
res,
loginAttemptCacheKey,
}: {
req: Request
res: Response
loginAttemptCacheKey: string
}) {
const sid = req.cookies[SESSION_COOKIE_NAME]

// Check if older session exists
if (sid) {
return this.redirectWithError(res, {
statusCode: 409, // Conflict
code: LOGIN_ATTEMPT_FAILED_ACTIVE_SESSION,
message: 'Multiple sessions detected!',
})
}

this.logger.warn(this.cacheService.createKeyError(loginAttemptCacheKey))

return this.redirectWithError(res)
}

/**
* Callback for the login flow
* This method is called from the identity server after the user has logged in
Expand All @@ -260,8 +304,8 @@ export class AuthService {
this.logger.error('Callback login IDS invalid request: ', idsError)

return this.redirectWithError(res, {
code: 500,
error: idsError,
statusCode: 500,
message: idsError,
})
}

Expand All @@ -273,7 +317,7 @@ export class AuthService {
)

return this.redirectWithError(res, {
code: 400,
statusCode: 400,
})
}

Expand All @@ -289,9 +333,11 @@ export class AuthService {
)

if (!loginAttemptData) {
this.logger.warn(this.cacheService.createKeyError(loginAttemptCacheKey))

return this.redirectWithError(res)
return this.handleMissingLoginAttempt({
req,
res,
loginAttemptCacheKey,
})
}

try {
Expand All @@ -304,11 +350,9 @@ export class AuthService {
const updatedTokenResponse = await this.updateTokenCache(tokenResponse)

// Clean up the login attempt from the cache since we have a successful login.
this.cacheService
.delete(this.cacheService.createSessionKeyType('attempt', query.state))
.catch((err) => {
this.logger.warn(err)
})
this.cacheService.delete(loginAttemptCacheKey).catch((err) => {
this.logger.warn(err)
})

// Clear any existing session cookie first
// This prevents multiple session cookies being set.
Expand Down Expand Up @@ -351,9 +395,7 @@ export class AuthService {
}
}

return res.redirect(
loginAttemptData.targetLinkUri || loginAttemptData.originUrl,
)
return res.redirect(loginAttemptData.targetLinkUri)
} catch (error) {
this.logger.error('Callback login failed: ', error)

Expand Down Expand Up @@ -394,8 +436,8 @@ export class AuthService {
)

return this.redirectWithError(res, {
code: 400,
error: 'Logout failed!',
statusCode: 400,
message: 'Logout failed!',
})
}

Expand Down
3 changes: 1 addition & 2 deletions apps/services/bff/src/app/modules/auth/auth.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ export type CachedTokenResponse = Omit<
}

export type LoginAttemptData = {
targetLinkUri?: string
targetLinkUri: string
codeVerifier: string
originUrl: string
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,10 @@ export class TokenRefreshService {
ttl: TokenRefreshService.MAX_POLL_TIME,
})

const newTokens = await this.idsService.refreshToken(
const newTokenCache = await this.idsService.refreshToken(
encryptedRefreshToken,
)
tokenResponse = await this.authService.updateTokenCache(newTokens)
tokenResponse = await this.authService.updateTokenCache(newTokenCache)
} catch (error) {
this.logger.warn('Failed to refresh tokens: ', error)
} finally {
Expand Down
17 changes: 11 additions & 6 deletions apps/services/bff/src/app/utils/create-error-query-str.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
export type CreateErrorQueryStrArgs = {
code: number
error: string
statusCode: number
message?: string
code?: string
}

/**
* This utility function creates a query string with the bff_error and bff_error_description parameters
* This utility function creates a query string with the error details
*/
export const createErrorQueryStr = ({
statusCode,
code,
error,
message,
}: CreateErrorQueryStrArgs) => {
return new URLSearchParams({
bff_error_code: code.toString(),
bff_error_description: error,
bff_error_status_code: statusCode.toString(),
...(message && {
bff_error_message: message,
}),
...(code && { bff_error_code: code }),
}).toString()
}
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,12 @@ export class InheritanceReportService extends BaseTemplateApiService {
attachments.push(
...(await Promise.all(
answers.heirsAdditionalInfoPrivateTransferFiles.map(async (file) => {
const content = await this.getFileContentBase64(file.key)
const filename = (
application.attachments as {
[key: string]: string
}
)[file.key]
const content = await this.getFileContentBase64(filename)
return {
name: file.name,
content,
Expand All @@ -128,7 +133,12 @@ export class InheritanceReportService extends BaseTemplateApiService {
attachments.push(
...(await Promise.all(
answers.heirsAdditionalInfoFilesOtherDocuments.map(async (file) => {
const content = await this.getFileContentBase64(file.key)
const filename = (
application.attachments as {
[key: string]: string
}
)[file.key]
const content = await this.getFileContentBase64(filename)
return {
name: file.name,
content,
Expand Down Expand Up @@ -178,7 +188,10 @@ export class InheritanceReportService extends BaseTemplateApiService {
)
return fileContent || ''
} catch (e) {
this.logger.warn('[estate]: Failed to get file content - ', e)
this.logger.warn(
'[inherhitance-report]: Failed to get file content - ',
e,
)
return 'err'
}
}
Expand Down
Loading

0 comments on commit d97d7ab

Please sign in to comment.