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

Forward headers for API routes? #608

Closed
mrkstwrt opened this issue Sep 18, 2020 · 6 comments · Fixed by #623
Closed

Forward headers for API routes? #608

mrkstwrt opened this issue Sep 18, 2020 · 6 comments · Fixed by #623

Comments

@mrkstwrt
Copy link

Is your feature request related to a problem? Please describe.
It doesn't seem possible to add to the whitelist for headers for anything other than defaults which is then overridden by the /api/* rules.

For example with this config:

inputs:
    cloudfront:
      /api/*:
        forward:
          headers: [ authorization ]

I get the error Setting custom cache behaviour for api/ route "/api/*" is not supported

Describe the solution you'd like
I want to be able to allow the authorization header to be passed up to my API routes, I can enable this by manually going into CloudFront and adding it to the whitelist, but this is overwritten every time I deploy so it's not really a workable solution when automating deployments. Or maybe I'm just doing it wrong.

@mrkstwrt
Copy link
Author

mrkstwrt commented Sep 18, 2020

EDIT:

So after more invesitgation it turns out I was going down the wrong path. It looks like what is actually happening is the authorization header is not forwarded for GET requests.

So the question now becomes, is this by design and is there any way to override this behaviour in the config?

@dphang
Copy link
Collaborator

dphang commented Sep 23, 2020

As you mentioned, I think you may need to forward Authorization header? I believe you can use: https://github.com/serverless-nextjs/serverless-next.js#custom-cloudfront-configuration. Or, manually, try to edit CloudFront cache behavior for /api for caching by headers and whitelist the Authorization header.

EDIT: ah, sorry, misread that you already tried that. I think that authorization not forwarded for GET requests is a CloudFront thing. From here: https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/RequestAndResponseBehaviorCustomOrigin.html

GET and HEAD requests – CloudFront removes the Authorization header field before forwarding the request to your origin.

OPTIONS requests – CloudFront removes the Authorization header field before forwarding the request to your origin if you configure CloudFront to cache responses to OPTIONS requests.

but if you cache based on header values and manually add authorization, I thought it should work according to docs.

As for why api custom cache behavior is not supported, will defer to @danielcondemarin. It seems we could remove this restriction as well?

@mrkstwrt
Copy link
Author

Yeah from what I've been able to gather through my testing/reading you are correct and like you say it is possible to manually add headers to the whitelist for GET and it works as expected.

The issue with this is that redeploying changes will revert this back, so it makes automated deployments more difficult as you need to do that manual step.

It would be nice if we could change the cache settings for api/* or be more specific like api/foo etc in order to keep the default caching except for routes where you need extra headers on GET requests such as Authorization in my case.

@dphang
Copy link
Collaborator

dphang commented Sep 24, 2020

Yup, I agree, another workaround for now is to use the aws cli or aws-sdk (for Node.js) to update the CloudFront deployment after the deployment. I will check if we can remove that restriction on modifying api caching behavior.

@danielcondemarin
Copy link
Contributor

danielcondemarin commented Sep 24, 2020

As for why api custom cache behavior is not supported, will defer to @danielcondemarin. It seems we could remove this restriction as well?

Correct, we decided not to allow custom config. for api/* in the past purely to reduce the API surface this PR opened. Basically the more we open up the api by allowing custom user input config. etc. the more difficult it becomes to maintain the project. Having said that I'm happy to remove the restriction 👍

@pavlelekic
Copy link

+1
@danielcondemarin I also want to set the custom headers, I need CloudFront-Viewer-City, CloudFront-Viewer-Country headers, I think those could be useful to others as well. It would be nice if we could choose which headers we want/need.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants