Skip to content
This repository has been archived by the owner on Feb 11, 2023. It is now read-only.

Externalise configuration and secrets #70

Open
wants to merge 69 commits into
base: master
Choose a base branch
from
Open

Conversation

apgrucza
Copy link

This pull request addresses issue #15, making it possible for the Lambda function to obtain its configuration from the AWS Systems Manager Parameter Store and AWS Secrets Manager. This makes the Lambda deployment package for an identity provider generic, therefore it can be pre-built for users to download (instead of them having to build it themselves).

I have made this feature available for Okta Native only, because that's the identity provider my organisation uses and the one that I could easily test with. Making the same changes for the other identity providers would be fairly straightforward but I have not done it due to the lack of unit tests in this project.

Functionality added:

  • Build script can now build a generic Lambda package for Okta Native
  • Build script can now build a Lambda package for rotation of the RSA key pair
  • Unit tests for new Lambda code (i.e. loading of configuration and rotation of key pair)
  • GitHub Actions workflow for CI (builds packages, runs unit tests and creates artifacts)
  • GitHub Actions workflow for releases (when a v* tag is pushed, creates a GitHub Release containing the Lambda packages)
  • README.md updated

Note that GitHub does not display the differences for build/build.js very well so it's best to use a different tool to view the differences for this file.

@payton payton self-requested a review June 20, 2020 18:42
@Freaky-namuH
Copy link

Hi @payton, Are you able to review this PR? Hopefully we can get it merged and not maintain our own fork.

Thank you!

@gwatts
Copy link

gwatts commented Jul 30, 2020

@apgrucza no doubt i'm missing something, but the additional dependencies blew up the size of the resulting bundle to 12MB, beyond the 1MB limit imposed by Lambda@Edge:

$ git clone https://github.com/iress/cloudfront-auth.git
Cloning into 'cloudfront-auth'...
remote: Enumerating objects: 626, done.
remote: Total 626 (delta 0), reused 0 (delta 0), pack-reused 626
Receiving objects: 100% (626/626), 196.69 KiB | 2.77 MiB/s, done.
Resolving deltas: 100% (334/334), done.
gareth@Dibbley tmp $ cd cloudfront-auth/
gareth@Dibbley cloudfront-auth (master=) $ ./build.sh okta_native

> [email protected] build /private/tmp/cloudfront-auth
> npm install && cd build && npm install && cd .. && node build/build.js "okta_native"

added 181 packages from 517 contributors and audited 182 packages in 3.732s

22 packages are looking for funding
  run `npm fund` for details

found 2 vulnerabilities (1 low, 1 high)
  run `npm audit fix` to fix them, or `npm audit` for details
npm WARN [email protected] No repository field.

added 49 packages from 39 contributors and audited 49 packages in 1.079s
found 1 low severity vulnerability
  run `npm audit fix` to fix them, or `npm audit` for details
Done... created Lambda function distributions/okta_native/okta_native.zip
gareth@Dibbley cloudfront-auth (master=) $ ls -lh distributions/okta_native/
total 24336
-rw-r--r--  1 gareth  wheel   261B Jul 30 18:13 auth.js
-rwxr-xr-x  1 gareth  wheel   591B Jul 30 18:13 code-challenge.js
-rw-r--r--  1 gareth  wheel   1.6K Jul 30 18:13 config.js
-rw-r--r--  1 gareth  wheel   789B Jul 30 18:13 config.json
-rwxr-xr-x  1 gareth  wheel    20K Jul 30 18:13 index.js
-rw-r--r--  1 gareth  wheel   457B Jul 30 18:13 nonce.js
-rw-r--r--  1 gareth  wheel    12M Jul 30 18:13 okta_native.zip
gareth@Dibbley cloudfront-auth (master=) $ 

@apgrucza
Copy link
Author

@gwatts it's because the build script you ran does not exclude devDependencies, so the node_modules directory contains modules that get unnecessarily included in the package.

You should not need to build the package yourself. The build is automated by a GitHub Actions workflow, which uses npm ci --production to ensure only the necessary modules get packaged. So you can just download okta_native_v2.0.1.zip from the Releases page: https://github.com/iress/cloudfront-auth/releases

If you do want to build the package yourself, you can call the build-ci script directly:

npm run-script build-ci okta_native

However as build.sh is the documented way to do custom builds (which are still required for the other identity providers), I will update it to use the new script too.

@gwatts
Copy link

gwatts commented Jul 31, 2020

@apgrucza hah was literally about to comment here that I realized only the ci build was using the --production flag

I'm building locally as I'm extending your work to add support for Google, since that's what I'm using

@apgrucza
Copy link
Author

Just added a commit - build.sh now excludes devDependencies too

@gwatts
Copy link

gwatts commented Aug 11, 2020

Would be nice to get this merged down; I have my own fork now at https://github.com/gwatts/cloudfront-auth/tree/gwatts/external-secrets which is a fork of @apgrucza branch and adds external secret support for G Suite along the same pattern

nbshetty and others added 8 commits October 5, 2020 09:59
Verify the access_token instead of the id_token.
Set the TOKEN cookie with the signed access_token and make it secure http only
Removed logic to re-sign the token using custom signing key.
* Merge Latest fix from origin repo  (#15)

* Very basic XSS prevention

* Parse `%20` as spaces before printing them

Co-authored-by: Payton Garland <[email protected]>
Co-authored-by: Thomas <[email protected]>

* Use the id_token nonce to validate.

Co-authored-by: Payton Garland <[email protected]>
Co-authored-by: Thomas <[email protected]>
PFS-1811  Add scope variable for AUTH_REQUEST
* PFS-1811 Update READ.ME

* Fix typo

Co-authored-by: Adrian Grucza <[email protected]>
@bsneider
Copy link

bsneider commented Sep 2, 2021

Is there a CFN template, or Terraform that creates the secrets in Systems Manager and deploys the lambdas?

@apgrucza
Copy link
Author

apgrucza commented Sep 4, 2021

Is there a CFN template, or Terraform that creates the secrets in Systems Manager and deploys the lambdas?

I do have Terraform for this. It's currently in a private repo but am looking at moving it here over the next week.

@bsneider
Copy link

bsneider commented Sep 8, 2021

@apgrucza the new tf code is awesome! I wonder if it could be finished off with creating the CloudFront distro too. Here is an example of the CloudFront being created with tf. https://github.com/oasys/terraform-aws-cloudfront-auth/blob/main/cloudfront.tf

@apgrucza
Copy link
Author

apgrucza commented Sep 9, 2021

@bsneider I didn't include a module for the CloudFront distribution because it would not add much value over the aws_cloudfront_distribution resource itself. I could add a CloudFront example to the examples folder though, if that's what you meant.

SiCoe and others added 30 commits September 22, 2023 13:48
include nodejs18.x in build targets
Configure NONCE and CV cookies as `secure` for pkce
All cookies sameSite as `Strict` for pkce
Set cookies as `secure` and `samesite: strict`in open github and openid
allows tooling to identify engine version
include Content-Type header in responses
Correct footers to correct repository url
This is due to licensing consern. 'html-entities' is distributed with the MIT license .
URI encode template replacement to avoid XSS
all `npm audit` vulnerability severities removed by updating referenced versions

note: some are breaking change updates
Version 4: Resolution of security (pen test) findings, dependancy vulnerabilities and other small updates
Whilst the app runs on nodejs18.x, the AWS SKD supplied is version 3.x and this app references version 2.x. A lambda layer can be supplied for this to work.
nodejs16.x comes bundled with v2.x of the AWS SDK, so no futher intervention is required.
default runtime to nodejs16.x
This enables authentication that isn't embeded into the same site.
e.g. changing host to Okta to log in, then back to site once authenticated.
Set SameSite cookie attribute to Lax for CV and NONCE
fix: stop redirect loop caused by TOKEN cookie not sent
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.