Skip to content

Conversation

@ndasari2
Copy link

@ndasari2 ndasari2 commented Nov 5, 2024

Summary of Changes

Today, the default behavior is an empty string if no value comes along in the aggregation or mobile requests for x-forwarded-for, which is being considered for originatingIpAddress in core. We rather pass the connector the public IpAddress as the originatingIpaddress in the RequestContext model.

MR that handles it in FHB accessor(as per client requirement): SAML single sign-on for MX Technologies · GitLab

We want to keep it generic so that it can be reusable for any such requirement in the future as well.

Fixes # (issue)

k8's https://gitlab.mx.com/mx/platform/k8s/-/merge_requests/4657/diffs

Public API Additions/Changes

Not related to a specific API

Downstream Consumer Impact

This is not a breaking change. No downstream consumer impact.

How Has This Been Tested?

Tested this change via path-accessor-q2-uux, which uses RequestContext.current().getOriginatingIP() in the authenticateWithUserKey scenario and made sure an ipaddress is always sent and is never null.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works

@ndasari2 ndasari2 self-assigned this Nov 5, 2024
@ndasari2 ndasari2 changed the title fix: checking and setting originatingIpAddress for requestContext fix: setting originatingIpAddress for requestContext Nov 5, 2024
@ndasari2 ndasari2 requested a review from mattnichols November 6, 2024 19:24
@ndasari2 ndasari2 marked this pull request as ready for review November 6, 2024 19:30
@meotch
Copy link

meotch commented Nov 7, 2024

Is this grabbing the requester's IP address (ie originatingIpAddress) or is this grabbing our service's IP address?

@ndasari2
Copy link
Author

ndasari2 commented Nov 7, 2024

Is this grabbing the requester's IP address (ie originatingIpAddress) or is this grabbing our service's IP address?

just our service's IP address in cases where the requester(grunt in this case) does not send their Ip address like on-demand jobs.

@meotch
Copy link

meotch commented Nov 7, 2024

Is there a way to fetch our IP internally without having to hit an external API like this?

@ndasari2
Copy link
Author

ndasari2 commented Nov 8, 2024

Is there a way to fetch our IP internally without having to hit an external API like this?

Not that I know of. @mattnichols do you think we can do that ?

@ndasari2 ndasari2 requested a review from mattnichols November 8, 2024 21:01
@mattnichols
Copy link
Collaborator

Can you clean up your commit history?

@mattnichols
Copy link
Collaborator

Is there a way to fetch our IP internally without having to hit an external API like this?

I don't think so.

@ndasari2
Copy link
Author

I am closing this MR to move this to a behavior instead.

@ndasari2 ndasari2 closed this Nov 14, 2024
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 this pull request may close these issues.

5 participants