Skip to content

Commit

Permalink
Merge branch 'master' into tech/EXUI-2249-content-security
Browse files Browse the repository at this point in the history
  • Loading branch information
connorpgpmcelroy authored Nov 27, 2024
2 parents a12a0b8 + 57c9c9e commit 7fca2c7
Show file tree
Hide file tree
Showing 8 changed files with 160 additions and 57 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
## [2.29.6](https://github.com/hmcts/rpx-xui-node-lib/compare/v2.29.5...v2.29.6) (2024-11-26)


### Bug Fixes

* Additional logging ([#253](https://github.com/hmcts/rpx-xui-node-lib/issues/253)) ([1402bd3](https://github.com/hmcts/rpx-xui-node-lib/commit/1402bd3d4395fb429be8ab60ff8d2ad2c45c6e53))

## [2.29.5](https://github.com/hmcts/rpx-xui-node-lib/compare/v2.29.4...v2.29.5) (2024-11-13)


Expand Down
36 changes: 21 additions & 15 deletions src/auth/models/strategy.class.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,13 @@ export abstract class Strategy extends events.EventEmitter {
const promise = new Promise((resolve) => {
if (req.session && this.options?.sessionKey) {
reqSession[this.options?.sessionKey] = { state }
this.logger.log('saving state in session')
req.session.save(() => {
this.logger.log('resolved promise, state saved')
this.logger.log('state saved in session')
resolve(true)
})
} else {
this.logger.warn('resolved promise, state not saved')
this.logger.warn('sessionKey not available state not saved')
resolve(false)
}
})
Expand All @@ -125,22 +126,24 @@ export abstract class Strategy extends events.EventEmitter {
(error: any, user: any, info: any) => {
/* istanbul ignore next */
if (error) {
this.logger.error('error => ', JSON.stringify(error))
this.logger.error('passport authenticate error ', JSON.stringify(error))
}
/* istanbul ignore next */
if (info) {
this.logger.info(info)
this.logger.info('passport authenticate info', JSON.stringify(info))
}
/* istanbul ignore next */
if (!user) {
const message = 'No user details returned by the authentication service, redirecting to login'
this.logger.log(message)
} else {
this.logger.log('User details from passport.authenticate ' + user.userInfo?.email)
}
},
)(req, res, next)
/* istanbul ignore next */
} catch (error) {
this.logger.error(error, this.strategyName)
this.logger.error('Exception in passport.authenticate', error, this.strategyName)
next(error)
return Promise.reject(error)
}
Expand Down Expand Up @@ -271,7 +274,7 @@ export abstract class Strategy extends events.EventEmitter {

if (!logMessages.length) return

this.logger.log(`emitAuthenticationFailure logMessages ${logMessages}`)
this.logger.log(`emitAuthenticationFailure logMessages ${logMessages.join('\n')}`)

res.locals.message = logMessages.join('\n')
this.emit(AUTH.EVENT.AUTHENTICATE_FAILURE, req, res, next)
Expand All @@ -284,16 +287,14 @@ export abstract class Strategy extends events.EventEmitter {
} as any,
(error: any, user: any, info: any) => {
const errorMessages: string[] = []
this.logger.log('inside passport authenticate')

this.logger.log('in passport authenticate callback')
if (error) {
switch (error.name) {
case 'TimeoutError':
const timeoutErrorMessage = `${error.name}: timeout awaiting ${error.url} for ${error.gotOptions.gotTimeout.request}ms`
errorMessages.push(timeoutErrorMessage)
this.logger.error(error)
break

default:
errorMessages.push(error)
this.logger.error(error)
Expand All @@ -305,8 +306,7 @@ export abstract class Strategy extends events.EventEmitter {
if (info.message === INVALID_STATE_ERROR) {
errorMessages.push(INVALID_STATE_ERROR)
}

this.logger.info(info)
this.logger.info('Authenticate callback info', info)
}

if (!user) {
Expand All @@ -317,7 +317,6 @@ export abstract class Strategy extends events.EventEmitter {
emitAuthenticationFailure(errorMessages)
return res.redirect(AUTH.ROUTE.LOGIN)
}

emitAuthenticationFailure(errorMessages)
this.verifyLogin(req, user, next, res)
},
Expand Down Expand Up @@ -404,16 +403,19 @@ export abstract class Strategy extends events.EventEmitter {
req.logIn(user, (err) => {
const roles = user.userinfo.roles
if (err) {
this.logger.error('verifyLogin error', err)
return next(err)
}
if (this.options.allowRolesRegex && !arrayPatternMatch(roles, this.options.allowRolesRegex)) {
this.logger.error(
`User has no application access, as they do not have a ${this.options.allowRolesRegex} role.`,
`User has no application access, as they do not have a role that matches ${this.options.allowRolesRegex}.`,
)
return this.logout(req, res)
}
if (!this.listenerCount(AUTH.EVENT.AUTHENTICATE_SUCCESS)) {
this.logger.log(`redirecting, no listener count: ${AUTH.EVENT.AUTHENTICATE_SUCCESS}`)
this.logger.log(
`redirecting, no listener count: ${AUTH.EVENT.AUTHENTICATE_SUCCESS}, user: ${user.email}`,
)
res.redirect(AUTH.ROUTE.DEFAULT_REDIRECT)
} else {
req.isRefresh = false
Expand Down Expand Up @@ -539,6 +541,7 @@ export abstract class Strategy extends events.EventEmitter {
done: (err: any, id?: any) => void,
): void => {
if (!this.listenerCount(eventName)) {
this.logger.error('no listeners for event ' + eventName)
done(null, eventObject)
} else {
this.emit(eventName, eventObject, done)
Expand All @@ -555,6 +558,7 @@ export abstract class Strategy extends events.EventEmitter {
const idamClient = options.clientID
return `grant_type=password&password=${userPassword}&username=${userName}&scope=${scope}&client_id=${idamClient}&client_secret=${clientSecret}`
}
const msg = 'options.routeCredential missing values'
throw new Error('options.routeCredential missing values')
}

Expand All @@ -563,6 +567,8 @@ export abstract class Strategy extends events.EventEmitter {
if (options.routeCredential) {
return `${options.logoutURL}/o/token`
}
throw new Error('missing routeCredential in options')
const msg = 'missing routeCredential in options'
this.logger.error('msg')
throw new Error(msg)
}
}
48 changes: 41 additions & 7 deletions src/auth/oauth2/models/oauth2.class.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,32 @@ describe('OAUTH2 Auth', () => {
},
}

xtest('it should be defined', () => {
test('it should be defined', () => {
expect(oauth2).toBeDefined()
})

xtest('it should be configurable', () => {
test('initialiseStrategy', () => {
const options = {
authorizationURL: 'someauthurl',
authorizationURL: 'http://localhost/someauthurl',
tokenURL: 'sometokenUrl',
clientID: 'client',
clientSecret: 'secret',
callbackURL: 'callbackUrl',
scope: 'scope',
sessionKey: 'node-lib',
useRoutes: false,
logoutURL: 'logoutUrl',
discoveryEndpoint: 'http://localhost/someEndpoint',
issuerURL: 'string',
responseTypes: [''],
tokenEndpointAuthMethod: 'string',
}
oauth2.initialiseStrategy(options)
expect(oauth2.isInitialised()).toBeTruthy()
})
test('it should be configurable', () => {
const options = {
authorizationURL: 'http://localhost/someauthurl',
tokenURL: 'sometokenUrl',
clientID: 'client',
clientSecret: 'secret',
Expand All @@ -60,7 +79,7 @@ describe('OAUTH2 Auth', () => {
expect(handler).toBeTruthy()
})

xtest('loginHandler with session and sessionKey', async () => {
test('loginHandler with session and sessionKey', async () => {
const mockRouter = createMock<Router>()
const options = createMock<AuthOptions>()
const logger = {
Expand All @@ -70,6 +89,9 @@ describe('OAUTH2 Auth', () => {
} as unknown as XuiLogger
options.sessionKey = 'test'
options.discoveryEndpoint = 'http://localhost/someEndpoint'
options.authorizationURL = 'http://localhost/someAuthorizationURL'
options.tokenURL = 'http://localhost/someTokenURL'
options.clientID = 'clientID1234'
const spy = jest.spyOn(passport, 'authenticate').mockImplementation(() => () => true)
const oAuth2 = new OAuth2(mockRouter, logger)
jest.spyOn(oAuth2, 'validateOptions')
Expand Down Expand Up @@ -97,8 +119,20 @@ describe('OAUTH2 Auth', () => {
expect(spy).toHaveBeenCalled()
})

xtest('loginHandler with session and no sessionKey', async () => {
test('loginHandler with session and no sessionKey', async () => {
const mockRouter = createMock<Router>()
const options = createMock<AuthOptions>()
options.sessionKey = 'test'
options.discoveryEndpoint = 'http://localhost/someEndpoint'
options.authorizationURL = 'http://localhost/someAuthorizationURL'
options.tokenURL = 'http://localhost/someTokenURL'
options.clientID = 'clientID1234'
options.callbackURL = 'http://localhost/callback'
options.clientSecret = 'secret123'
options.issuerURL = 'http://localhost/someEndpoint'
options.logoutURL = 'http://localhost/testUrl'
options.tokenEndpointAuthMethod = 'string'
options.scope = 'periscope'
const logger = {
log: jest.fn(),
error: jest.fn(),
Expand Down Expand Up @@ -128,7 +162,7 @@ describe('OAUTH2 Auth', () => {
expect(spy).toHaveBeenCalled()
})

xtest('setCallbackURL', () => {
test('setCallbackURL', () => {
const mockRequest = {
...mockRequestRequired,
body: {},
Expand All @@ -148,7 +182,7 @@ describe('OAUTH2 Auth', () => {
expect(next).toHaveBeenCalled()
})

xtest('setHeaders should set auth headers', () => {
test('setHeaders should set auth headers', () => {
const roles = ['test', 'test1']
const authToken = 'Bearer abc123'
const mockRequest = {
Expand Down
5 changes: 5 additions & 0 deletions src/auth/oauth2/models/oauth2.class.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,16 @@ export class OAuth2 extends Strategy {
}

public initialiseStrategy = async (authOptions: AuthOptions): Promise<void> => {
this.logger.log('initialiseStrategy start')
const options = this.getOAuthOptions(authOptions)
passport.use(this.strategyName, new XUIOAuth2Strategy(options, this.verify))
this.logger.log('initialiseStrategy end')
}

public isInitialised(): boolean {
return passport.strategies != null
}

public verify = (
accessToken: string,
refreshToken: string,
Expand Down
Loading

0 comments on commit 7fca2c7

Please sign in to comment.