feature/cloudfront/sign: allow setting cookie expiration in CookieOptions

[soggycactus]

This PR is a simple change that allows end users to specify the cookie expiration through CookieOptions. This makes it easier to return a persistent cookie to the client with an expiration equal to that used in a Canned Policy.

[soggycactus]

@skmcgrail it seems I'm unable to add any reviewers - mind if I get some eyes on this PR? Added you because I saw that you review most of these

[soggycactus]

bumping @jasdel @skotambkar as well

[skmcgrail]

Out of curiosity what is the typical relation to the cookie expiration time, and the content policy expiration time in practice. I see that the intention of this PR is to add the option for setting cookie expiration time. Is there a time where the canned policy expiration time, and the cookie expiration time should be different? Should the expiration time already passed into the CookieSigner#Sign be used on the cookie if a boolean flag like SetExpiration is set to true on CookieOptions?

[skmcgrail]

To follow up as well I see https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/private-content-setting-signed-cookie-canned-policy.html notes that Expires & Max-Age is recommended to not be included. Would it be better to add support for the CloudFront-Expires cookie that is outlined here better meet your use case?

[soggycactus]

Out of curiosity what is the typical relation to the cookie expiration time, and the content policy expiration time in practice. I see that the intention of this PR is to add the option for setting cookie expiration time. Is there a time where the canned policy expiration time, and the cookie expiration time should be different? Should the expiration time already passed into the CookieSigner#Sign be used on the cookie if a boolean flag like SetExpiration is set to true on CookieOptions?

To follow up as well I see https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/private-content-setting-signed-cookie-canned-policy.html notes that Expires & Max-Age is recommended to not be included. Would it be better to add support for the CloudFront-Expires cookie that is outlined here better meet your use case?

The link you included pretty much explains the use case - session cookies are wiped if you close a browser, while persistent cookies (created by specifying either Expires or Max-age) aren't wiped until they expire. Persistent cookies can provide a better user experience (they last between browser sessions), but do provide a slightly greater security risk - the same risk as having persistent authentication cookies to bypass logging into your favorite sites.

The relationship between the canned policy expiration time and the cookie expiration is one I would think of as: cookie expiration <= canned policy expiration. In a lot of cases I would think it'd be the same, but I could see a use case where perhaps you want to give a user access to a VOD for an entire week, but only allow their cookies to exist for a few hours, forcing them to re-authenticate after a certain time. The cookie expiration shouldn't ever be greater than the policy expiration - it'd be quite useless because you'd just get hit with a 403 anyways.

The Cloudfront-Expires cookie only pertains to requests made to Cloudfront using a canned policy; they'll get hit with a 403 if the expiration time has passed and Cloudfront will not show the content. It doesn't have any effect on whether the signed cookies themselves last between browser sessions.

All that being said, I opened this PR while I was implementing Cloudfront for my backend because I noticed it'd be cleaner to have all the cookie properties set through one interface - it's trivial to set the cookie expiration by hand (an additional 2 lines of code maybe?), so I don't see this PR as recommending persistent cookies over session cookies. It just gives the end-users the ability to create both entirely through the AWS SDK. If we want to force users to do it by hand because AWS recommends only using session cookies, we can close this out, that's fine by me :)

[soggycactus]

Closing this out since it's gone stale