Skip to content
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

feat: add a support for onUnauthenticatedRequest property #7780

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,9 @@ layout: Doc
## `bin/serverless`

Please use `bin/serverless.js` instead. `bin/serverless` will be removed with v2.0.0

<a name="AWS_ALB_ALLOW_UNAUTHENTICATED"><div>&nbsp;</div></a>

## AWS ALB `allowUnauthenticated`

Please use `onUnauthenticatedRequest` instead. `allowUnauthenticated` will be removed with v2.0.0
4 changes: 2 additions & 2 deletions docs/providers/aws/events/alb.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ provider:
userPoolArn: 'arn:aws:cognito-idp:us-east-1:123412341234:userpool/us-east-1_123412341', # required
userPoolClientId: '1h57kf5cpq17m0eml12EXAMPLE', # required
userPoolDomain: 'your-test-domain' # required
allowUnauthenticated: true # If set to true this allows the request to be forwarded to the target when user is not authenticated. Omit this parameter to make a HTTP 401 Unauthorized error be returned instead
onUnauthenticatedRequest: 'deny' # If set to 'allow' this allows the request to be forwarded to the target when user is not authenticated. When omitted it defaults 'deny' which makes a HTTP 401 Unauthorized error be returned. Alternatively configure to 'authenticate' to redirect request to IdP authorization endpoint.
requestExtraParams: # optional. The query parameters (up to 10) to include in the redirect request to the authorization endpoint
prompt: 'login'
redirect: false
Expand All @@ -93,7 +93,7 @@ provider:
issuer: 'https://www.iamscam.com', # required. The OIDC issuer identifier of the IdP. This must be a full URL, including the HTTPS protocol, the domain, and the path
tokenEndpoint: 'http://somewhere.org', # required
userInfoEndpoint: 'https://another-example.com' # required
allowUnauthenticated: true # If set to true this allows the request to be forwarded to the target when user is not authenticated. Omit this parameter to make a HTTP 401 Unauthorized error be returned instead
onUnauthenticatedRequest: 'deny' # If set to 'allow' this allows the request to be forwarded to the target when user is not authenticated. When omitted it defaults 'deny' which makes a HTTP 401 Unauthorized error be returned. Alternatively configure to 'authenticate' to redirect request to IdP authorization endpoint.
requestExtraParams:
prompt: 'login'
redirect: false
Expand Down
4 changes: 2 additions & 2 deletions docs/providers/aws/guide/serverless.yml.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ provider:
userPoolArn: 'arn:aws:cognito-idp:us-east-1:123412341234:userpool/us-east-1_123412341', # required
userPoolClientId: '1h57kf5cpq17m0eml12EXAMPLE', # required
userPoolDomain: 'your-test-domain' # required
allowUnauthenticated: true # If set to true this allows the request to be forwarded to the target when user is not authenticated. Omit this parameter to make a HTTP 401 Unauthorized error be returned instead
onUnauthenticatedRequest: 'deny' # If set to 'allow' this allows the request to be forwarded to the target when user is not authenticated. When omitted it defaults 'deny' which makes a HTTP 401 Unauthorized error be returned. Alternatively configure to 'authenticate' to redirect request to IdP authorization endpoint.
requestExtraParams: # optional. The query parameters (up to 10) to include in the redirect request to the authorization endpoint
prompt: 'login'
redirect: false
Expand All @@ -104,7 +104,7 @@ provider:
issuer: 'https://www.iamscam.com', # required. The OIDC issuer identifier of the IdP. This must be a full URL, including the HTTPS protocol, the domain, and the path
tokenEndpoint: 'http://somewhere.org', # required
userInfoEndpoint: 'https://another-example.com' # required
allowUnauthenticated: true # If set to true this allows the request to be forwarded to the target when user is not authenticated. Omit this parameter to make a HTTP 401 Unauthorized error be returned instead
onUnauthenticatedRequest: 'deny' # If set to 'allow' this allows the request to be forwarded to the target when user is not authenticated. Omit or set to 'deny' (default) to make a HTTP 401 Unauthorized error be returned instead. Alternatively configure to 'authenticate' to redirect request to IdP authorization endpoint.
requestExtraParams:
prompt: 'login'
redirect: false
Expand Down
209 changes: 209 additions & 0 deletions lib/plugins/aws/package/compile/events/alb/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ const sinon = require('sinon');
const AwsProvider = require('../../../../provider/awsProvider');
const AwsCompileAlbEvents = require('./index');
const Serverless = require('../../../../../../Serverless');
const runServerless = require('../../../../../../../tests/utils/run-serverless');
const fixtures = require('../../../../../../../tests/fixtures');

describe('AwsCompileAlbEvents', () => {
let awsCompileAlbEvents;
Expand Down Expand Up @@ -79,4 +81,211 @@ describe('AwsCompileAlbEvents', () => {
return awsCompileAlbEvents.hooks['package:compileEvents']();
});
});

describe('onUnauthenticatedRequest', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@medikoo I thought before I start writing a set of tests to migrate them to use runServerless I will check what would like to change in the example test that tests the change to onUnauthenticatedRequest.

Do you expect to have an assertion to https://github.com/serverless/serverless/pull/7780/files#diff-f210397c5bba20a4235eaae4251556f9R131 ?

Or

Should the generated cloudformation be asserted as that is the final result?

I would probably opt-out for the latter one.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally if we confirm on generated template. So something as:

expect(resource.Properties.StageName).to.equal('$default');
which will validate that we have "deny" set at given property.

Still if for some reason you'll find troublesome getting right path to resource to be validated, let me know

let cfResources;
let naming;

const serverlessConfiguration = (authorizerConfigOverride = {}) => ({
provider: {
alb: {
authorizers: {
test: Object.assign(
{
type: 'cognito',
userPoolClientId: 'userPoolClientId',
userPoolArn: 'userPoolArn',
userPoolDomain: 'userPoolDomain',
scope: 'openid',
sessionCookieName: 'sessionCookie',
},
authorizerConfigOverride
),
},
},
},
functions: {
trigger: {
events: [
{
alb: {
listenerArn:
'arn:aws:elasticloadbalancing:' +
'us-east-1:123456789012:listener/app/my-load-balancer/' +
'50dc6c495c0c9188/f2f7dc8efc522ab2',
conditions: {
path: '/',
},
priority: 1,
authorizer: 'test',
},
},
],
},
},
});

const baseAuthenticateCognitoConfig = (override = {}) =>
Object.assign(
{
UserPoolArn: 'userPoolArn',
UserPoolClientId: 'userPoolClientId',
UserPoolDomain: 'userPoolDomain',
OnUnauthenticatedRequest: 'deny',
Scope: 'openid',
SessionCookieName: 'sessionCookie',
AuthenticationRequestExtraParams: undefined,
SessionTimeout: undefined,
},
override
);

it('should be "deny" by default', () =>
fixtures
.extend('functionDestinations', serverlessConfiguration())
.then(fixturePath =>
runServerless({
cwd: fixturePath,
cliArgs: ['package'],
})
)
.then(serverless => {
({ Resources: cfResources } = serverless.service.provider.compiledCloudFormationTemplate);
naming = serverless.getProvider('aws').naming;
})
.then(() => {
const albListenerRuleLogicalId = naming.getAlbListenerRuleLogicalId('trigger', 1);
const rule = cfResources[albListenerRuleLogicalId];

expect(rule.Type).to.equal('AWS::ElasticLoadBalancingV2::ListenerRule');
expect(rule.Properties.Actions).to.have.length(2);
expect(rule.Properties.Actions[0].Type).to.equal('authenticate-cognito');
expect(rule.Properties.Actions[0].Order).to.equal(1);
expect(rule.Properties.Actions[0].AuthenticateCognitoConfig).to.deep.equal(
baseAuthenticateCognitoConfig()
);
}));

it('maps "allowUnauthenticated" set to true to "allow"', () =>
fixtures
.extend('functionDestinations', serverlessConfiguration({ allowUnauthenticated: true }))
.then(fixturePath =>
runServerless({
cwd: fixturePath,
cliArgs: ['package'],
})
)
.then(serverless => {
({ Resources: cfResources } = serverless.service.provider.compiledCloudFormationTemplate);
naming = serverless.getProvider('aws').naming;
})
.then(() => {
const albListenerRuleLogicalId = naming.getAlbListenerRuleLogicalId('trigger', 1);
const rule = cfResources[albListenerRuleLogicalId];

expect(rule.Properties.Actions[0].AuthenticateCognitoConfig).to.deep.equal(
baseAuthenticateCognitoConfig({ OnUnauthenticatedRequest: 'allow' })
);
}));

it('"allowUnauthenticated" set to false should be ineffective', () =>
fixtures
.extend('functionDestinations', serverlessConfiguration({ allowUnauthenticated: false }))
.then(fixturePath =>
runServerless({
cwd: fixturePath,
cliArgs: ['package'],
})
)
.then(serverless => {
({ Resources: cfResources } = serverless.service.provider.compiledCloudFormationTemplate);
naming = serverless.getProvider('aws').naming;
})
.then(() => {
const albListenerRuleLogicalId = naming.getAlbListenerRuleLogicalId('trigger', 1);
const rule = cfResources[albListenerRuleLogicalId];

expect(rule.Properties.Actions[0].AuthenticateCognitoConfig).to.deep.equal(
baseAuthenticateCognitoConfig({ OnUnauthenticatedRequest: 'deny' })
);
}));

it('supports setting value to "allow"', () =>
fixtures
.extend(
'functionDestinations',
serverlessConfiguration({ onUnauthenticatedRequest: 'allow' })
)
.then(fixturePath =>
runServerless({
cwd: fixturePath,
cliArgs: ['package'],
})
)
.then(serverless => {
({ Resources: cfResources } = serverless.service.provider.compiledCloudFormationTemplate);
naming = serverless.getProvider('aws').naming;
})
.then(() => {
const albListenerRuleLogicalId = naming.getAlbListenerRuleLogicalId('trigger', 1);
const rule = cfResources[albListenerRuleLogicalId];

expect(rule.Properties.Actions[0].AuthenticateCognitoConfig).to.deep.equal(
baseAuthenticateCognitoConfig({ OnUnauthenticatedRequest: 'allow' })
);
}));

it('supports setting value to "authenticate"', () =>
fixtures
.extend(
'functionDestinations',
serverlessConfiguration({ onUnauthenticatedRequest: 'authenticate' })
)
.then(fixturePath =>
runServerless({
cwd: fixturePath,
cliArgs: ['package'],
})
)
.then(serverless => {
({ Resources: cfResources } = serverless.service.provider.compiledCloudFormationTemplate);
naming = serverless.getProvider('aws').naming;
})
.then(() => {
const albListenerRuleLogicalId = naming.getAlbListenerRuleLogicalId('trigger', 1);
const rule = cfResources[albListenerRuleLogicalId];

expect(rule.Properties.Actions[0].AuthenticateCognitoConfig).to.deep.equal(
baseAuthenticateCognitoConfig({ OnUnauthenticatedRequest: 'authenticate' })
);
}));

it('takes precedence over allowUnauthenticated', () =>
fixtures
.extend(
'functionDestinations',
serverlessConfiguration({
onUnauthenticatedRequest: 'deny',
allowUnauthenticated: true,
})
)
.then(fixturePath =>
runServerless({
cwd: fixturePath,
cliArgs: ['package'],
})
)
.then(serverless => {
({ Resources: cfResources } = serverless.service.provider.compiledCloudFormationTemplate);
naming = serverless.getProvider('aws').naming;
})
.then(() => {
const albListenerRuleLogicalId = naming.getAlbListenerRuleLogicalId('trigger', 1);
const rule = cfResources[albListenerRuleLogicalId];

expect(rule.Properties.Actions[0].AuthenticateCognitoConfig).to.deep.equal(
baseAuthenticateCognitoConfig({ OnUnauthenticatedRequest: 'deny' })
);
}));
});
});
17 changes: 16 additions & 1 deletion lib/plugins/aws/package/compile/events/alb/lib/validate.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';

const _ = require('lodash');
const logDeprecation = require('../../../../../../../utils/logDeprecation');

// eslint-disable-next-line max-len
const CIDR_IPV6_PATTERN = /^s*((([0-9A-Fa-f]{1,4}:){7}([0-9A-Fa-f]{1,4}|:))|(([0-9A-Fa-f]{1,4}:){6}(:[0-9A-Fa-f]{1,4}|((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3})|:))|(([0-9A-Fa-f]{1,4}:){5}(((:[0-9A-Fa-f]{1,4}){1,2})|:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3})|:))|(([0-9A-Fa-f]{1,4}:){4}(((:[0-9A-Fa-f]{1,4}){1,3})|((:[0-9A-Fa-f]{1,4})?:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){3}(((:[0-9A-Fa-f]{1,4}){1,4})|((:[0-9A-Fa-f]{1,4}){0,2}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){2}(((:[0-9A-Fa-f]{1,4}){1,5})|((:[0-9A-Fa-f]{1,4}){0,3}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){1}(((:[0-9A-Fa-f]{1,4}){1,6})|((:[0-9A-Fa-f]{1,4}){0,4}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(:(((:[0-9A-Fa-f]{1,4}){1,7})|((:[0-9A-Fa-f]{1,4}){0,5}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:)))(%.+)?s*(\/([0-9]|[1-9][0-9]|1[0-1][0-9]|12[0-8]))$/;
Expand Down Expand Up @@ -217,7 +218,21 @@ module.exports = {
},

validateAlbAuth(auth) {
auth.onUnauthenticatedRequest = auth.allowUnauthenticated ? 'allow' : 'deny';
const hasAllowUnauthenticated = auth.allowUnauthenticated != null;
const hasOnUnauthenticatedRequest = auth.onUnauthenticatedRequest != null;

if (hasAllowUnauthenticated) {
logDeprecation(
'AWS_ALB_ALLOW_UNAUTHENTICATED',
'allowUnauthenticated is deprecated, use onUnauthenticatedRequest instead'
);
}

if (hasAllowUnauthenticated && !hasOnUnauthenticatedRequest) {
auth.onUnauthenticatedRequest = auth.allowUnauthenticated ? 'allow' : 'deny';
} else {
auth.onUnauthenticatedRequest = auth.onUnauthenticatedRequest || 'deny';
}

return auth;
},
Expand Down
28 changes: 0 additions & 28 deletions lib/plugins/aws/package/compile/events/alb/lib/validate.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -441,32 +441,4 @@ describe('#validate()', () => {
);
});
});

describe('#validateAlbAuth()', () => {
it('returns valid authorizer when valid object provided', () => {
const auth = {
type: 'cognito',
userPoolId: 'userPoolId',
};
expect(awsCompileAlbEvents.validateAlbAuth(auth, 'authName')).to.deep.equal({
type: 'cognito',
userPoolId: 'userPoolId',
onUnauthenticatedRequest: 'deny',
});
});

it('returns valid authorizer when valid object provided with allowUnauthenticated', () => {
const auth = {
type: 'oidc',
clientSecret: 'i-am-secret',
allowUnauthenticated: true,
};
expect(awsCompileAlbEvents.validateAlbAuth(auth, 'authName')).to.deep.equal({
type: 'oidc',
clientSecret: 'i-am-secret',
allowUnauthenticated: true,
onUnauthenticatedRequest: 'allow',
});
});
});
});