Zappa icon indicating copy to clipboard operation
Zappa copied to clipboard

Added support for passing the client source IP address from API Gateway.

Open nickovs opened this issue 3 years ago • 7 comments

Description

This is a minor PR to check if the requestContext object provided by the API Gateway includes an identity object and that contains a sourceIP value representing the client IP address; if present then that value is made available to the application in the CLIENT_SOURCE_IP environment variable.

The PR also adds a test case for passing the client IP address.

GitHub Issues

This addresses issue #470.

nickovs avatar Apr 12 '23 23:04 nickovs

@nickovs

I'd like to get a release out soon, and get this in.

Can you run make black, make isort on this? Looks like the linter is failing.

monkut avatar Aug 14 '23 04:08 monkut

@monkut I've cleaned up the white space so that make black makes no changes. isort seemed happy so no imports were changed.

nickovs avatar Aug 14 '23 14:08 nickovs

Coverage Status

coverage: 74.626% (+0.02%) from 74.605% when pulling b7092f413435100601aeb1a4415897dc0bdcdf7c on nickovs:feature-pass-sourcip into 26d6182129ecd737cbef95ea8d462132a8fb3c58 on zappa:master.

coveralls avatar Aug 17 '23 04:08 coveralls

Just re-reviewing this, and I believe that the client IP is available in "REMOTE_ADDR".

https://github.com/zappa/Zappa/blob/master/zappa/wsgi.py#L89-L96

This isn't the lambda context "sourceIp", but I suspect it should be the desired value.

monkut avatar Aug 17 '23 04:08 monkut

@monkut Unfortunately the AWS application gateway doesn't properly set the X-Forwarded-For header, so the default deployment process for Zappa ends up with that header being empty. To see this try creating a new Zappa deployment for the following Flask app:

import os
from flask import Flask
app = Flask(__name__)

@app.route("/", methods=["GET", "POST"])
def lambda_handler(event=None, context=None):
    return "Remote IP is: {}".format(os.environ.get("REMOTE_ADDR"))

if __name__ == "__main__":
    app.run(debug=True)

When you access the page you will get: Remote IP is: None

nickovs avatar Aug 17 '23 15:08 nickovs

Having said that, it might make sense to change the PR to put the result into REMOTE_ADDR instead of CLIENT_SOURCE_IP, so that app developers only have one place to look.

If you agree to that then I can make the change. Based on my observation so far, it looks we will never have both code paths executed in the same deployment, but if you have an opinion as to if the new code should override the old code or not in the event of a conflict then let me know.

nickovs avatar Aug 17 '23 16:08 nickovs

@nickovs

Ok, I checked and confirmed that I am using the REMOTE_ADDR value it a project.
If it's not available by default from APIGW, I suspect that I configured the APIGW to allow the header through.

In that case can you update this PR to:

  • Use REMOTE_ADDR for the client ip.
  • Fallback to the context "sourceIp" if the X-Forwarded-For is unavailable.

monkut avatar Aug 18 '23 01:08 monkut

Hi there! Unfortunately, this PR has not seen any activity for at least 90 days. If the PR is still relevant to the latest version of Zappa, please comment within the next 10 days if you wish to keep it open. Otherwise, it will be automatically closed.

github-actions[bot] avatar Apr 03 '24 18:04 github-actions[bot]

Hi there! Unfortunately, this PR was automatically closed as it had not seen any activity in at least 100 days. If the PR is still relevant to the latest version of Zappa, please open a new PR.

github-actions[bot] avatar Apr 13 '24 20:04 github-actions[bot]