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

Issue with IP Blocking and Allowing in global configuration #2165

Open
CavidH opened this issue Oct 7, 2024 · 7 comments · May be fixed by #2170
Open

Issue with IP Blocking and Allowing in global configuration #2165

CavidH opened this issue Oct 7, 2024 · 7 comments · May be fixed by #2170
Assignees
Labels
Autumn'24 Autumn 2024 release Configuration Ocelot feature: Configuration proposal Proposal for a new functionality in Ocelot Security Options Ocelot feature: Security Options
Milestone

Comments

@CavidH
Copy link

CavidH commented Oct 7, 2024

I configured IP blocking and allowing in Ocelot using SecurityOptions, but it's not working.

{
  "GlobalConfiguration": {
    "BaseUrl": "http://localhost:5000",
    "SecurityOptions": {
      "IPBlockedList": ["192.168.0.23"]
    }
  }
}


The IP blocking configuration is not working as expected.

@raman-m raman-m added Security Options Ocelot feature: Security Options question Initially seen a question could become a new feature or bug or closed ;) Configuration Ocelot feature: Configuration labels Oct 8, 2024
@raman-m
Copy link
Member

raman-m commented Oct 8, 2024

Hello, Cavid!
It seems we lack support for global settings.
The potential solutions could be:

  1. Solely using ocelot.json. Define the options for each route individually.
  2. Utilizing C# coding. Replace the ISecurityOptionsCreator service in the DI container by redeveloping the SecurityOptionsCreator class to consider only the global settings.
    Services.TryAddSingleton<ISecurityOptionsCreator, SecurityOptionsCreator>();

Which solution would be more convenient for you?

@raman-m
Copy link
Member

raman-m commented Oct 8, 2024

Hello @Fabman08,
The absence of global settings support is a significant issue. Here's the current usage of SecurityOptionsCreator:

var securityOptions = _securityOptionsCreator.Create(fileRoute.SecurityOptions);

Consequently, the method should accept two arguments, including global settings:

public SecurityOptions Create(FileSecurityOptions securityOptions)

Would you be able to allocate some time to address this?

@raman-m raman-m changed the title Issue with IP Blocking and Allowing in Ocelot Configuration Issue with IP Blocking and Allowing in global configuration Oct 8, 2024
@Fabman08
Copy link
Contributor

Fabman08 commented Oct 8, 2024

Hi @raman-m!
Sure, I'll be able to fix the issue this or next week. ☺️

@CavidH thank you for reporting this issue! 👍

@raman-m raman-m added proposal Proposal for a new functionality in Ocelot and removed question Initially seen a question could become a new feature or bug or closed ;) labels Oct 8, 2024
@CavidH
Copy link
Author

CavidH commented Oct 8, 2024

Thank you very much for your help. We will eagerly await the new version.

@CavidH
Copy link
Author

CavidH commented Oct 8, 2024

Hello, Cavid! It seems we lack support for global settings. The potential solutions could be:

  1. Solely using ocelot.json. Define the options for each route individually.
  2. Utilizing C# coding. Replace the ISecurityOptionsCreator service in the DI container by redeveloping the SecurityOptionsCreator class to consider only the global settings.
    Services.TryAddSingleton<ISecurityOptionsCreator, SecurityOptionsCreator>();

Which solution would be more convenient for you?

I had thought of the first option, but since I have too many services, it will be difficult to control this

@Fabman08
Copy link
Contributor

@raman-m just a little question about Global Settings behaviours.

I've found three ways to fix the issue:

  1. Every properties in Route FileSecurityOptions overrides their Global Settings values
    eg.
  • Global:
    Allowed: B,C
    Blocked: E
    ExcludeAllowedFromBlocked: true
  • Route:
    Allowed: A,B,C
    Blocked: D,E,F
    ExcludeAllowedFromBlocked: false
  • Final:
    Allowed: A,B,C
    Blocked: D,E,F
    ExcludeAllowedFromBlocked: false
  1. Only the existing properties in Route FileSecurityOptions overrides their Global Settings values
    eg.
  • Global:
    Allowed: B,C
    Blocked: E
    ExcludeAllowedFromBlocked: true
  • Route:
    Allowed: A,B,C
    Blocked: null
    ExcludeAllowedFromBlocked: null
  • Final:
    Allowed: A,B,C
    Blocked: E
    ExcludeAllowedFromBlocked: true
  1. The Route FileSecurityOptions values will be merged to Global Settings values
    eg.
  • Global:
    Allowed: A,B,C
    Blocked: D,E
    ExcludeAllowedFromBlocked: true
  • Route:
    Allowed: F,G,H
    Blocked: I,K
    ExcludeAllowedFromBlocked: false
  • Final:
    Allowed: A,B,C,F,G,H
    Blocked: D,E,I,K
    ExcludeAllowedFromBlocked: false

Which one is the best?

@raman-m
Copy link
Member

raman-m commented Oct 10, 2024

@Fabman08 Your research is too complicated!

Which one is the best?

It is best to prioritize the route-specific FileSecurityOptions object over the global settings. Therefore, if route settings are defined, all global settings should be disregarded. In practice, if the route object is null, then the global settings should be utilized.

Every properties in Route FileSecurityOptions overrides their Global Settings values

Yes, if route options are defined, all global ones should be ignored. We will highlight this in the documentation. There should be no merging whatsoever. This approach is the simplest and most correct solution, as merging properties would be erroneous due to potential conflicts within the algorithm. Therefore, merging is identified as a source of bugs.

@Fabman08 Fabman08 linked a pull request Oct 11, 2024 that will close this issue
@raman-m raman-m added the Oct'24 October 2024 release label Oct 11, 2024
@raman-m raman-m added this to the October'24 milestone Oct 11, 2024
@raman-m raman-m added Autumn'24 Autumn 2024 release and removed Oct'24 October 2024 release labels Oct 26, 2024
@raman-m raman-m modified the milestones: October'24, Autumn'24 Oct 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Autumn'24 Autumn 2024 release Configuration Ocelot feature: Configuration proposal Proposal for a new functionality in Ocelot Security Options Ocelot feature: Security Options
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants