joostvanderborg
commented
2 years ago projenrc projen versie is niet gepinned
Check, aangepast.
'@aws-cdk/aws-apigatewayv2-alpha', '@aws-cdk/aws-apigatewayv2-integrations-alpha', Zijn allebei nogal Alpha en kan tot breaking change leiden bij update van cdk
Klopt, alle apigatewayv2-constructs voor cdk v2 zijn nog alpha. Daaarom risico voor nu genomen omdat het in productie geen issues oplevert, alleen bij nieuwe releases, en het ombouwwerk naar schatting beperkt zal blijven als het nodig is.
new SSM.StringParameter(this, 'ssm_auth_2', { moet die niet in een secret iets?
Nee, de OpenIDConnect clientid wordt in elk request zichtbaar in de browser meegegeven, is geen geheime informatie.
Moet de sessionstable niet van een kms key voorzien worden?
Jawel, stat issue voor open.
allowedMethods: AllowedMethods.ALLOW_ALL, Idd alle methods nodig?
In ieder geval ook POST, en de opties zijn GET, HEAD, GET, HEAD, PUT en alles.
deze is nodig denk ik: minimumProtocolVersion: cloudfront.SecurityPolicyProtocol.TLS_V1_2_2019, //see: https://docs.aws.amazon.com /AmazonCloudFront/latest/DeveloperGuide/secure-connections-supported-viewer-protocols-ciphers.html Toegevoegd.
de logbucket is niet encrypt
Wel encrypt, maar niet met eigen key. Staat issue voor open.
xssProtection: { protection: true, modeBlock: true, reportUri: 'https://example.com/csp-report', override: true }, nog een example url (moeten we geen eigen csp-report endpoint opzetten, al dan niet door team-online?)
Goeie vraag. Voor deze header report-uri verwijderd, kan niet eens tegelijk met modeBlock. (Lijkt me een documentatie-issue met CDK, als ik even heb om dat te dubbelchecken 's kijken of ik een doc-PR bij de CDK kan doen :).
Lambda's. session.js bevat this.dbClient = new DynamoDBClient(); Beter om die in de index.js buiten de exports.handler op te nemen (misschien eerst even testen hoe performance nu is) zelfde voor const secretsManagerClient = new SecretsManagerClient();
Helemaal mee eens, maar inderdaad nog geen prio aan gegeven. Wel wat ideeën hoe het handig kan, en in m'n POC ook mee getest. Voor nu is in accp performance van de http-requests de bottleneck.
in OpenIdConnect.js daar ook nog omdat er limieten zitten aan aantal requests/sec naar parameter store/secretsmanager
Goeie tip, ook issue voor aangemaakt.
in de /home lambda doe je 2 awaits voor uitkering en brp api; die zouden toch ook tegelijk kunnen, en dus in 1 promise?
Die was al gewijzigd, worden nu inderdaad simultaan gedaan :).
Echt goed inhoudelijk beoordelen van de lambda's op (security) issues kan ik niet.
Algemeen: alle code is lekker gestructureerd en goed leesbaar. Ik zou hier vrij snel aanpassingen in kunnen en durven doen.
Dank, goed om te horen!
projenrc
projen versie is niet gepinned
'@aws-cdk/aws-apigatewayv2-alpha', '@aws-cdk/aws-apigatewayv2-integrations-alpha', Zijn allebei nogal Alpha en kan tot breaking change leiden bij update van cdk
new SSM.StringParameter(this, 'ssm_auth_2', {
Moet de sessionstable niet van een kms key voorzien worden?
allowedMethods: AllowedMethods.ALLOW_ALL, Idd alle methods nodig?
cloudfront: deze is nodig denk ik: minimumProtocolVersion: cloudfront.SecurityPolicyProtocol.TLS_V1_2_2019, //see: https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/secure-connections-supported-viewer-protocols-ciphers.html
de logbucket is niet encrypt
xssProtection: { protection: true, modeBlock: true, reportUri: 'https://example.com/csp-report', override: true },
Lambda's. session.js bevat this.dbClient = new DynamoDBClient(); Beter om die in de index.js buiten de exports.handler op te nemen (misschien eerst even testen hoe performance nu is)
zelfde voor const secretsManagerClient = new SecretsManagerClient(); in OpenIdConnect.js daar ook nog omdat er limieten zitten aan aantal requests/sec naar parameter store/secretsmanager
in de /home lambda doe je 2 awaits voor uitkering en brp api; die zouden toch ook tegelijk kunnen, en dus in 1 promise?
Echt goed inhoudelijk beoordelen van de lambda's op (security) issues kan ik niet.
Algemeen: alle code is lekker gestructureerd en goed leesbaar. Ik zou hier vrij snel aanpassingen in kunnen en durven doen.