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

Evolve oidc logout #87

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,13 @@ When NGINX Plus is deployed behind another proxy, the original protocol and port
* Set the **redirect URI** to the address of your NGINX Plus instance (including the port number), with `/_codexch` as the path, e.g. `https://my-nginx.example.com:443/_codexch`
* Ensure NGINX Plus is configured as a confidential client (with a client secret) or a public client (with PKCE S256 enabled)
* Make a note of the `client ID` and `client secret` if set
* Set the **logout redirect URI** to the address of your NGINX Plus instance (including the port number), with `/_logout` as the path, e.g. `https://my-nginx.example.com:443/_logout`

* If your IdP supports OpenID Connect Discovery (usually at the URI `/.well-known/openid-configuration`) then use the `configure.sh` script to complete configuration. In this case you can skip the next section. Otherwise:
* Obtain the URL for `jwks_uri` or download the JWK file to your NGINX Plus instance
* Obtain the URL for the **authorization endpoint**
* Obtain the URL for the **token endpoint**
* Obtain the URL for the **logout endpoint**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd replace logout endpoint with end session endpoint, since this is most likely how it will be reflected in the IdP documentation.


## Configuring NGINX Plus

Expand All @@ -111,7 +113,7 @@ Manual configuration involves reviewing the following files so that they match y

* **openid_connect_configuration.conf** - this contains the primary configuration for one or more IdPs in `map{}` blocks
* Modify all of the `map…$oidc_` blocks to match your IdP configuration
* Modify the URI defined in `map…$oidc_logout_redirect` to specify an unprotected resource to be displayed after requesting the `/logout` location
* Modify the URI defined in `map…$redir_post_logout` to specify an unprotected resource to be displayed after requesting the `/logout` location
* Set a unique value for `$oidc_hmac_key` to ensure nonce values are unpredictable
* If NGINX Plus is deployed behind another proxy or load balancer, modify the `map…$redirect_base` and `map…$proto` blocks to define how to obtain the original protocol and port number.

Expand All @@ -125,7 +127,7 @@ Manual configuration involves reviewing the following files so that they match y

* **openid_connect.server_conf** - this is the NGINX configuration for handling the various stages of OpenID Connect authorization code flow
* No changes are usually required here
* Modify the `resolver` directive to match a DNS server that is capable of resolving the IdP defined in `$oidc_token_endpoint`
* Modify the `resolver` directive to match a DNS server that is capable of resolving the IdP defined in `$oidc_token_endpoint` and `$oidc_logout_endpoint`
* If using [`auth_jwt_key_request`](http://nginx.org/en/docs/http/ngx_http_auth_jwt_module.html#auth_jwt_key_request) to automatically fetch the JWK file from the IdP then modify the validity period and other caching options to suit your IdP

* **openid_connect.js** - this is the JavaScript code for performing the authorization code exchange and nonce hashing
Expand Down
4 changes: 2 additions & 2 deletions configure.sh
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ fi
# Build an intermediate configuration file
# File format is: <NGINX variable name><space><IdP value>
#
jq -r '. | "$oidc_authz_endpoint \(.authorization_endpoint)\n$oidc_token_endpoint \(.token_endpoint)\n$oidc_jwks_uri \(.jwks_uri)"' < /tmp/${COMMAND}_$$_json > /tmp/${COMMAND}_$$_conf
jq -r '. | "$oidc_authz_endpoint \(.authorization_endpoint)\n$oidc_token_endpoint \(.token_endpoint)\n$oidc_logout_endpoint \(.logout_endpoint)\n$oidc_jwks_uri \(.jwks_uri)"' < /tmp/${COMMAND}_$$_json > /tmp/${COMMAND}_$$_conf
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. In most implementations with which I'm familiar, the endpoint is called end_session_endpoint. I’m curious, what implementation of OP did you work with, where is the logout_endpoint used? In addition, the OpenID Connect Discovery 1.0 refers specifically to end_session_endpoint, and there is not a single mention of logout_endpoint.
  2. The end_session_endpoint parameter is optional and many implementations do not support it, but you do not handle these cases in any way.


# Create a random value for HMAC key, adding to the intermediate configuration file
echo "\$oidc_hmac_key `openssl rand -base64 18`" >> /tmp/${COMMAND}_$$_conf
Expand Down Expand Up @@ -178,7 +178,7 @@ fi

# Loop through each configuration variable
echo "$COMMAND: NOTICE: Configuring $CONFDIR/openid_connect_configuration.conf"
for OIDC_VAR in \$oidc_authz_endpoint \$oidc_token_endpoint \$oidc_jwt_keyfile \$oidc_hmac_key $CLIENT_ID_VAR $CLIENT_SECRET_VAR $PKCE_ENABLE_VAR; do
for OIDC_VAR in \$oidc_authz_endpoint \$oidc_token_endpoint \$oidc_logout_endpoint \$oidc_jwt_keyfile \$oidc_hmac_key $CLIENT_ID_VAR $CLIENT_SECRET_VAR $PKCE_ENABLE_VAR; do
# Pull the configuration value from the intermediate file
VALUE=`grep "^$OIDC_VAR " /tmp/${COMMAND}_$$_conf | cut -f2 -d' '`
echo -n "$COMMAND: NOTICE: - $OIDC_VAR ..."
Expand Down
23 changes: 15 additions & 8 deletions openid_connect.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
/*
* JavaScript functions for providing OpenID Connect with NGINX Plus
*
*
* Copyright (C) 2020 Nginx, Inc.
*/
var newSession = false; // Used by oidcAuth() and validateIdToken()

export default {auth, codeExchange, validateIdToken, logout};
export default {auth, codeExchange, validateIdToken, logout, redirectPostLogout};

function retryOriginalRequest(r) {
delete r.headersOut["WWW-Authenticate"]; // Remove evidence of original failed auth_jwt
Expand Down Expand Up @@ -51,7 +51,7 @@ function auth(r, afterSyncCheck) {
r.return(302, r.variables.oidc_authz_endpoint + getAuthZArgs(r));
return;
}

// Pass the refresh token to the /_refresh location so that it can be
// proxied to the IdP in exchange for a new id_token
r.subrequest("/_refresh", "token=" + r.variables.refresh_token,
Expand Down Expand Up @@ -266,10 +266,17 @@ function validateIdToken(r) {

function logout(r) {
r.log("OIDC logout for " + r.variables.cookie_auth_token);
r.variables.session_jwt = "-";
r.variables.access_token = "-";
r.variables.refresh_token = "-";
r.return(302, r.variables.oidc_logout_redirect);
var logoutArgs = "?post_logout_redirect_uri=" + r.variables.redirect_base + r.variables.oidc_logout_redirect + "&id_token_hint=" + r.variables.session_jwt;

r.variables.session_jwt = '-';
r.variables.access_token = '-';
r.variables.refresh_token = '-';
r.return(302, r.variables.oidc_logout_endpoint + logoutArgs);
}

// Redirect URL after logged-out from the IDP.
function redirectPostLogout(r) {
r.return(302, r.variables.redir_post_logout);
}

function getAuthZArgs(r) {
Expand Down Expand Up @@ -311,5 +318,5 @@ function idpClientAuth(r) {
return "code=" + r.variables.arg_code + "&code_verifier=" + r.variables.pkce_code_verifier;
} else {
return "code=" + r.variables.arg_code + "&client_secret=" + r.variables.oidc_client_secret;
}
}
}
21 changes: 13 additions & 8 deletions openid_connect.server_conf
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

location = /_jwks_uri {
internal;
proxy_cache jwk; # Cache the JWK Set recieved from IdP
proxy_cache jwk; # Cache the JWK Set received from IdP
proxy_cache_valid 200 12h; # How long to consider keys "fresh"
proxy_cache_use_stale error timeout updating; # Use old JWK Set if cannot reach IdP
proxy_ssl_server_name on; # For SNI to the IdP
Expand All @@ -29,9 +29,9 @@
# This location is called by the IdP after successful authentication
status_zone "OIDC code exchange";
js_content oidc.codeExchange;
error_page 500 502 504 @oidc_error;
error_page 500 502 504 @oidc_error;
}

location = /_token {
# This location is called by oidcCodeExchange(). We use the proxy_ directives
# to construct the OpenID Connect token request, as per:
Expand Down Expand Up @@ -67,16 +67,21 @@
}

location = /logout {
# This location is called by UI to handle OIDC logout with IDP
status_zone "OIDC logout";
add_header Set-Cookie "auth_token=; $oidc_cookie_flags"; # Send empty cookie
add_header Set-Cookie "auth_redir=; $oidc_cookie_flags"; # Erase original cookie
js_content oidc.logout;
}

location = /_logout {
# This location is the default value of $oidc_logout_redirect (in case it wasn't configured)
default_type text/plain;
return 200 "Logged out\n";
# This location is a default value of $oidc_logout_redirect called by the
# IDP after closing user session in the IDP.

# Clean cookies
add_header Set-Cookie "auth_nonce=; $oidc_cookie_flags"; # Send empty cookie
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the practical meaning of your approach when you initiate security context destruction at the time of the /logout call, but clear the cookies only in the callback (when the UA has returned from the OP)?

add_header Set-Cookie "auth_token=; $oidc_cookie_flags"; # Erase original cookie
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These comments make no sense, so I'd remove them.

add_header Set-Cookie "auth_redir=; $oidc_cookie_flags";

js_content oidc.redirectPostLogout;
}

location @oidc_error {
Expand Down
14 changes: 11 additions & 3 deletions openid_connect_configuration.conf
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,16 @@ map $host $oidc_jwt_keyfile {
default "http://127.0.0.1:8080/auth/realms/master/protocol/openid-connect/certs";
}

map $host $oidc_logout_endpoint {
default "http://127.0.0.1:8080/auth/realms/master/protocol/openid-connect/logout";
}

map $host $redir_post_logout {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some doubts about the practical usefulness of this functionality for several reasons:

  1. The description of redir_post_logout and oidc_logout_redirect is a little confusing and makes me think that they are about the same thing.
  2. Why do we need a redirect somewhere else when the UA has successfully landed in a /_logout location? At this point, the user can be shown text about the successful logout, replace it with an app unprotected page, or implement any other logic within this location.

# Where to send browser after requesting /logout location. This can be
# replaced with a custom logout page, or complete URL.
default "http://127.0.0.1:8080";
}

map $host $oidc_client {
default "my-client-id";
}
Expand All @@ -45,9 +55,7 @@ map $host $oidc_scopes {
}

map $host $oidc_logout_redirect {
# Where to send browser after requesting /logout location. This can be
# replaced with a custom logout page, or complete URL.
default "/_logout"; # Built-in, simple logout page
default "/_logout"; # This is called by IdP after successful logout.
}

map $host $oidc_hmac_key {
Expand Down