-
Notifications
You must be signed in to change notification settings - Fork 64
Harmony 1980 - Support for external validation services #707
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.
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
Changes from all commits
f30b214
9694fc5
d03ae50
71e2517
d46d8d6
7941da4
f792823
f8c6ff0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| import axios from 'axios'; | ||
| import { NextFunction, Response } from 'express'; | ||
|
|
||
| import HarmonyRequest from '../models/harmony-request'; | ||
| import { ExternalValidationError } from '../util/errors'; | ||
|
|
||
| /** | ||
| * Middleware to validate users against an external endpoint configured for a service. | ||
| * @param req - The client request, containing an operation | ||
| * @param res - The client response | ||
| * @param next - The next function in the middleware chain | ||
| */ | ||
| export async function externalValidation( | ||
| req: HarmonyRequest, res: Response, next: NextFunction, | ||
| ): Promise<void> { | ||
| const { operation, context } = req; | ||
| const url = context?.serviceConfig?.external_validation_url; | ||
| if (!url) return next(); | ||
|
|
||
| req.context.logger.info('timing.external-validation.start'); | ||
| const startTime = new Date().getTime(); | ||
| try { | ||
| await axios.post( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like to add an info level timing message for calling the validation URL (see the other places we log
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
| url, | ||
| operation, | ||
| { | ||
| headers: { | ||
| 'Authorization': `Bearer: ${req.accessToken}`, | ||
| }, | ||
| }, | ||
| ); | ||
| } catch (e) { | ||
| req.context.logger.error('External validation failed'); | ||
| if (e.response) { | ||
| req.context.logger.error(`Validation status: ${e.response.status}`); | ||
| req.context.logger.error(`Validation response: ${JSON.stringify(e.response.data, null, 2)}`); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: log the response status here as well.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
| return next(new ExternalValidationError(e.response.data, e.response.status)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should log the response body and status code the validation URL returned - I didn't see either in the logs.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
| } else { | ||
| req.context.logger.error(`Error calling validation endpoint: ${url}.`); | ||
| return next(e); | ||
| } | ||
| } finally { | ||
| const durationMs = new Date().getTime() - startTime; | ||
| req.context.logger.info('timing.external-validation.end', { durationMs }); | ||
| } | ||
|
|
||
| return next(); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if we should try to future proof it to allow for multiple validations and different types of validations. e.g.
It's probably fine to keep it just a single URL for now and change it if needed in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think we will keep it simple for now and support those use cases as needed.