Skip to content

Conversation

gwatts
Copy link

@gwatts gwatts commented Aug 3, 2020

@iress given your PR is still pending, perhaps it'd be worth adding my Google update to it (feedback welcome)

This commit extends the generic package support to include Google
authenticators

It generates separate packages for "hosted-domain" and "email-lookup"
validation types, but does not yet include one for "google-groups" as
that requires a JSON file at runtime; need to determine a good option to
supply that as a secret.

This commit extends the generic package support to include Google
authenticators

It generates separate packages for "hosted-domain" and "email-lookup"
validation types, but does not yet include one for "google-groups" as
that requires a JSON file at runtime; need to determine a good option to
supply that as a secret.
case 'groups-lookup':
config.SUBDIST = isGeneric && "groups_lookup";
if (isGeneric) {
// TODO: provide a base64 encoded method for user-supplied google-authz.json

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be nice, i guess, if you wanted to support groups-based lookups.. not something i currently need though

const getParametersByPathPromise = ssm.getParametersByPath({ Path: `/${name}`, WithDecryption: true }).promise();

// Get key pair from Secrets Manager
const secretsmanager = new aws.SecretsManager({ region: 'us-east-1' });

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should the region be configurable?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lambda@Edge functions must be deployed in us-east-1, so i'm not sure there's any value in making the region configurable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants