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

Make Nginx auth_request module to be able to expose auth error body when needed #11182

Closed
d-fal opened this issue Apr 1, 2024 · 3 comments
Closed
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@d-fal
Copy link

d-fal commented Apr 1, 2024

Nginx auth_request module is being shipped by Nginx and it is being used in ingress-nginx [nginx, nginx-1.25 ]. This module, IMO, has a limitation which is lack of on-demand transparency when requests are not successfully authenticated.

This issue also is related to this very same limitation of Nginx auth_request module. It also demands to expose auth error response body to the end user.

I have modified nginx auth_request module to let authentication error body expose to the caller. Here is the link : https://github.com/d-fal/nginx-module-dev-guide/tree/main/contrib/auth-module-with-body

with this change, ingress-nginx will have a new boolean directive called auth_request_mask_body which lets Nginx to be transparent about auth_request module auth error body.

When being used, it works as follow:

     location = /auth {
          internal;
          proxy_pass http://auth-service.local;
     }

     location /protected {
          auth_request /auth;
          # auth_request_mask_body controls transparency of Nginx auth_request module when it comes to error messages.
          # when auth_request_mask_body: off Nginx sends auth error response to the end user.
          # auth_request_mask_body: on Nginx shows a predefined 401 error page.
          auth_request_mask_body off; # default on
          # ...
     }

This module can be incorporated in ingress-nginx and proper annotations can be added accordingly. What do you think?

@d-fal d-fal added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 1, 2024
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority labels Apr 1, 2024
@strongjz
Copy link
Member

strongjz commented Apr 1, 2024

It would be better to maintain this project and the module if you upstream that change into the nginx module.

/close

@k8s-ci-robot
Copy link
Contributor

@strongjz: Closing this issue.

In response to this:

It would be better to maintain this project and the module if you upstream that change into the nginx module.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
Development

No branches or pull requests

3 participants