Skip to content

Commit

Permalink
clean up logging a bit
Browse files Browse the repository at this point in the history
  • Loading branch information
@andywilkinshmcts
andywilkinshmcts committed Aug 23, 2024
1 parent 6c829f5 commit 280e30b
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 27 deletions.
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@hmcts/rpx-xui-node-lib",
"version": "2.29.2-output-stdout",
"version": "2.29.3-logging",
"description": "Common nodejs library components for XUI",
"main": "dist/index",
"types": "dist/index.d.ts",
Expand Down
2 changes: 1 addition & 1 deletion src/auth/messaging.constants.ts
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export const VERIFY_ERROR_MESSAGE_NO_ACCESS_ROLES = 'User does not have any access roles.'
export const VERIFY_ERROR_MESSAGE_NO_ACCESS_ROLES = 'User does not have any IDAM roles.'
27 changes: 13 additions & 14 deletions src/auth/models/strategy.class.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export abstract class Strategy extends events.EventEmitter {
resolve(true)
})
} else {
this.logger.warn('resolved promise, state not saved')
this.logger.warn('resolved promise, no session key, state not saved')
resolve(false)
}
})
Expand All @@ -123,13 +123,14 @@ export abstract class Strategy extends events.EventEmitter {
state,
} as any,
(error: any, user: any, info: any) => {
this.logger.log('authenticate callback user', JSON.stringify(user));

Check failure on line 126 in src/auth/models/strategy.class.ts

View workflow job for this annotation

GitHub Actions / build 

Delete `;`

Check failure on line 126 in src/auth/models/strategy.class.ts

View workflow job for this annotation

GitHub Actions / build 

Delete `;`
/* istanbul ignore next */
if (error) {
this.logger.error('error => ', JSON.stringify(error))
this.logger.error('authenticate callback error ', JSON.stringify(error))
}
/* istanbul ignore next */
if (info) {
this.logger.info(info)
this.logger.info('authenticate callback',info)

Check failure on line 133 in src/auth/models/strategy.class.ts

View workflow job for this annotation

GitHub Actions / build 

Insert `·`

Check failure on line 133 in src/auth/models/strategy.class.ts

View workflow job for this annotation

GitHub Actions / build 

Insert `·`
}
/* istanbul ignore next */
if (!user) {
Expand All @@ -140,7 +141,7 @@ export abstract class Strategy extends events.EventEmitter {
)(req, res, next)
/* istanbul ignore next */
} catch (error) {
this.logger.error(error, this.strategyName)
this.logger.error('authentication exception', error, this.strategyName)
next(error)
return Promise.reject(error)
}
Expand Down Expand Up @@ -196,13 +197,13 @@ export abstract class Strategy extends events.EventEmitter {
}

const redirect = req.query.redirect ? req.query.redirect : AUTH.ROUTE.LOGIN
this.logger.log('redirecting to => ', redirect)
this.logger.log('logout redirecting to ', redirect)
// 401 is when no accessToken
res.redirect(redirect as string)

/* istanbul ignore next */
} catch (e) {
this.logger.error('error => ', e)
this.logger.error('logout exception ', e)
res.status(401).redirect(AUTH.ROUTE.DEFAULT_REDIRECT)
}
this.logger.log('logout end')
Expand Down Expand Up @@ -262,7 +263,7 @@ export abstract class Strategy extends events.EventEmitter {

/* istanbul ignore next */
public callbackHandler = (req: Request, res: Response, next: NextFunction): void => {
this.logger.log('inside callbackHandler')
this.logger.log('inside callbackHandler for ', req.originalUrl);

Check failure on line 266 in src/auth/models/strategy.class.ts

View workflow job for this annotation

GitHub Actions / build 

Delete `;`

Check failure on line 266 in src/auth/models/strategy.class.ts

View workflow job for this annotation

GitHub Actions / build 

Delete `;`
const INVALID_STATE_ERROR = 'Invalid authorization request state.'
const reqSession = req.session as MySessionData

Expand All @@ -284,8 +285,7 @@ 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':
Expand All @@ -305,8 +305,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('callbackHandler authenticate info', info)
}

if (!user) {
Expand All @@ -315,9 +314,9 @@ export abstract class Strategy extends events.EventEmitter {
this.logger.log(message)

emitAuthenticationFailure(errorMessages)
this.logger.info('redirecting to ' + AUTH.ROUTE.LOGIN);

Check failure on line 317 in src/auth/models/strategy.class.ts

View workflow job for this annotation

GitHub Actions / build 

Delete `;`

Check failure on line 317 in src/auth/models/strategy.class.ts

View workflow job for this annotation

GitHub Actions / build 

Delete `;`
return res.redirect(AUTH.ROUTE.LOGIN)
}

emitAuthenticationFailure(errorMessages)
this.verifyLogin(req, user, next, res)
},
Expand Down Expand Up @@ -403,12 +402,12 @@ export abstract class Strategy extends events.EventEmitter {
}
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, roles ${roles.join(' ')} don't match ${this.options.allowRolesRegex} role.`,

Check failure on line 405 in src/auth/models/strategy.class.ts

View workflow job for this annotation

GitHub Actions / build 

Replace `this.options.allowRolesRegex` with `⏎························this.options.allowRolesRegex⏎····················`

Check failure on line 405 in src/auth/models/strategy.class.ts

View workflow job for this annotation

GitHub Actions / build 

Replace `this.options.allowRolesRegex` with `⏎························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(`no listener count, redirecting to: ${AUTH.ROUTE.DEFAULT_REDIRECT}`)
res.redirect(AUTH.ROUTE.DEFAULT_REDIRECT)
} else {
req.isRefresh = false
Expand Down
22 changes: 11 additions & 11 deletions src/auth/oidc/models/openid.class.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,16 +108,17 @@ export class OpenID extends AuthStrategy {
reqsession.passport.user.tokenset = this.convertTokenSet(tokenSet)

if (!this.listenerCount(AUTH.EVENT.AUTHENTICATE_SUCCESS)) {
this.logger.log(`refresh: no listener count: ${AUTH.EVENT.AUTHENTICATE_SUCCESS}`)
this.logger.log(`refresh: no listeners: ${AUTH.EVENT.AUTHENTICATE_SUCCESS}`)
return next()
} else {
req.isRefresh = true
this.logger.log('refresh: success')
this.emit(AUTH.EVENT.AUTHENTICATE_SUCCESS, req, res, next)
return
}
}
} catch (e) {
this.logger.error('refresh error => ', e)
this.logger.error('refresh exception ', e)
next(e)
}
}
Expand Down Expand Up @@ -160,7 +161,7 @@ export class OpenID extends AuthStrategy {
this.logger.warn(VERIFY_ERROR_MESSAGE_NO_ACCESS_ROLES)
return done(null, false, { message: VERIFY_ERROR_MESSAGE_NO_ACCESS_ROLES })
}
this.logger.info('verify okay, user:', userinfo)
this.logger.info('verify success, user:', userinfo)

return done(null, { tokenset: this.convertTokenSet(tokenset), userinfo })
}
Expand Down Expand Up @@ -234,7 +235,7 @@ export class OpenID extends AuthStrategy {
if (req.session && this.options?.sessionKey) {
reqsession[this.options?.sessionKey] = { state }
req.session.save(() => {
this.logger.log('resolved promise, nonce & state saved')
this.logger.log('resolved promise, state saved')
resolve(true)
})
} else {
Expand All @@ -246,7 +247,7 @@ export class OpenID extends AuthStrategy {
try {
await promise

this.logger.log('calling passport authenticate')
this.logger.log('OAuth2 calling passport authenticate')

return passport.authenticate(
this.strategyName,
Expand All @@ -256,28 +257,27 @@ export class OpenID extends AuthStrategy {
state,
} as any,
(error: any, user: any, info: any) => {
this.logger.log('passport authenticate')

this.logger.log('OAuth2 passport authenticate callback')
if (error) {
this.logger.error('loginHandler error: ', JSON.stringify(error))
}
/* istanbul ignore next */
if (info) {
this.logger.info(info)
this.logger.info('OAuth2 info', info)
}
/* istanbul ignore next */
if (user) {
const message = 'loginHandler User details returned by passport authenticate'
const message = 'OAuth2 loginHandler User details from passport authenticate'
this.logger.log(message)
}
if (!user) {
const message = 'loginHandler no User details returned by passport authenticate'
const message = 'OAuth2 loginHandler no User details returned by passport authenticate'
this.logger.log(message)
}
},
)(req, res, next)
} catch (error) {
this.logger.error('this should not throw an error')
this.logger.error('OAuth2 loginHandler unexpected error', error)
throw new Error(`this should not throw an ${error}`)
}
}
Expand Down

0 comments on commit 280e30b

Please sign in to comment.