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

Add OIDC userinfo endpoint for User-Agent to obtain claims about the End-User #76

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

shawnhankim
Copy link
Contributor

@shawnhankim shawnhankim commented Dec 22, 2022

Issue:

Background:
As a Product Manager (PM) of NGINX Management Suite - API Connectivity Management (NMS-ACM) or a PM of SPA,
I want a User-Agent to obtain claims about End-User to show signed-in user information in detail using the OIDC userinfo endpoint of the OP.

Description:

  • Added a map variable of $oidc_userinfo_endpoint
  • Added a config example of obtaining claims about End-User using the oidc_userinfo_endpoint.

@shawnhankim
Copy link
Contributor Author

@route443 :

  • Thanks for your review in detail for the PR.
  • This PR is to simplify from the previous PR.
  • For you to easily manage this repo to reduce any concerns from the enhancements based on the reviews on the PR, I have divided a big PR into small PRs, and this is one of PRs.
  • Let me know if you have any questions.

@shawnhankim shawnhankim changed the title feat: add userinfo endpoint add userinfo endpoint Dec 30, 2022
@shawnhankim shawnhankim changed the title add userinfo endpoint Add userinfo endpoint Dec 30, 2022
@route443
Copy link
Contributor

route443 commented Jan 4, 2023

Information from Userinfo endpoint is returned to the client (nginx) and the client should be able to use it, for example in the form of variables. This cannot be done with the proposed patch => it doesn't bring any benefit to the OIDC reference implementation.
Secondly, it can mislead users, we declare that we support Userinfo endpoint, but there is no way to access the final keys and their values.
Your current goals for getting Userinfo from the User Agent are outside of the OIDC specification and outside of our reference implementation, so I'd recommend to use the following config:

    location = /foobar {
        error_page 401 = @do_oidc_flow;
        proxy_intercept_errors on;
        proxy_ssl_server_name on;
        proxy_set_header Authorization "Bearer $access_token";
        proxy_pass $oidc_userinfo_endpoint;
    }

@shawnhankim
Copy link
Contributor Author

Information from Userinfo endpoint is returned to the client (nginx) and the client should be able to use it, for example in the form of variables. This cannot be done with the proposed patch => it doesn't bring any benefit to the OIDC reference implementation. Secondly, it can mislead users, we declare that we support Userinfo endpoint, but there is no way to access the final keys and their values. Your current goals for getting Userinfo from the User Agent are outside of the OIDC specification and outside of our reference implementation, so I'd recommend to use the following config:

    location = /foobar {
        error_page 401 = @do_oidc_flow;
        proxy_intercept_errors on;
        proxy_ssl_server_name on;
        proxy_set_header Authorization "Bearer $access_token";
        proxy_pass $oidc_userinfo_endpoint;
    }
  • FYI. This feature was originally requested by the PM in the team who manage this OIDC repo before you started joining your team. Hence, it was firstly used to NGINX Management Suite - API Connectivity Manager, and the PM's plan was to donate here.
  • However, as the goal and benefit per product can be always changed and enhanced based on other aspects, I respect your suggestion. I will try thinking of how to enhance based on your comments. Thanks for your suggestion.

@shawnhankim shawnhankim force-pushed the 023-userinfo branch 2 times, most recently from fedccf9 to 5441a04 Compare January 9, 2023 22:03
remove /userinfo location

Add userinfo endpoint in README.md
@shawnhankim shawnhankim changed the title Add userinfo endpoint Add userinfo endpoint for User-Agent to obtain claims about the End-User Jan 9, 2023
@shawnhankim shawnhankim changed the title Add userinfo endpoint for User-Agent to obtain claims about the End-User Add OIDC userinfo endpoint for User-Agent to obtain claims about the End-User Jan 9, 2023
@shawnhankim
Copy link
Contributor Author

@route443 :

  • Thanks for your opinion.
  • The previous /userinfo location is removed and is going to separately used as the App level reference implementation in the other repo as it is the requirement of NMS-ACM's requirement.
  • Revised it as this repo is to provide simpler reference implementation for a proxy instead of application.
  • Let me know if I miss anything.

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.

2 participants