From d02da70366bc4495f640bad695c220c083674a3c Mon Sep 17 00:00:00 2001 From: Ivan Ovchinnikov Date: Fri, 5 Sep 2025 23:55:45 +0000 Subject: [PATCH] Handle NameID Format in Logout messages. Previously, the SAML SP code did not consider the NameID Format during Single Logout. This commit fixes the logout handling to be compliant with the SAML 2.0 specifications regarding NameID. --- saml_sp.js | 22 +++++++++++++++------- t/js_saml.t | 19 ++++++++++++++++--- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/saml_sp.js b/saml_sp.js index 0a052d7..2218e7f 100644 --- a/saml_sp.js +++ b/saml_sp.js @@ -100,6 +100,7 @@ async function handleSAMLMessage(messageType, r) { /* Define necessary parameters needed to create a SAML LogoutResponse */ opt.nameID = nameID[0]; + opt.nameIDFormat = nameID[1]; opt.inResponseTo = id; opt.relayState = params.RelayState; @@ -659,7 +660,8 @@ async function produceSAMLMessage(messageType, r, opt) { break; case "LogoutResponse": /* Obtain the status code for the LogoutResponse message */ - opt.statusCode = getLogoutStatusCode(r.variables.saml_name_id, opt.nameID) + opt.statusCode = getLogoutStatusCode(r.variables.saml_name_id, opt.nameID, + r.variables.saml_name_id_format, opt.nameIDFormat); break; } @@ -701,14 +703,17 @@ function setAuthRedirCookie(r) { ]; } -function getLogoutStatusCode(sessionNameID, requestNameID) { - /* If no session exists, return Logout Success */ +function getLogoutStatusCode(sessionNameID, requestNameID, sessionFormat, requestFormat) { + /* If no session exists, treat as already logged out */ if (!sessionNameID || sessionNameID === '-') { return 'urn:oasis:names:tc:SAML:2.0:status:Success'; } - - /* If session exists, return Logout Success if NameID matches */ - return requestNameID === sessionNameID + /* Treat missing formats as "unspecified" for comparison */ + const defaultFmt = 'urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified'; + const spFormat = (!sessionFormat || sessionFormat === '-') ? defaultFmt : sessionFormat; + const requestFmt = (!requestFormat || requestFormat === '-') ? defaultFmt : requestFormat; + /* Only return Success if both NameID value and Format match exactly */ + return (requestNameID === sessionNameID && spFormat === requestFmt) ? 'urn:oasis:names:tc:SAML:2.0:status:Success' : 'urn:oasis:names:tc:SAML:2.0:status:Requester'; } @@ -722,7 +727,9 @@ async function createSAMLMessage(opt, id, messageType) { nameIDPolicy: ``, }), LogoutRequest: () => ({ - nameID: `${opt.nameID}`, + nameID: opt.nameIDFormat && opt.nameIDFormat !== 'urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified' + ? `${opt.nameID}` + : `${opt.nameID}`, }), LogoutResponse: () => ({ inResponseTo: ` InResponseTo="${opt.inResponseTo}"`, @@ -1234,6 +1241,7 @@ function parseConfigurationOptions(r, messageType) { } opt.relayState = r.variables.saml_logout_landing_page; opt.nameID = r.variables.saml_name_id; + opt.nameIDFormat = r.variables.saml_name_id_format; opt.allowedClockSkew = validateClockSkew('saml_allowed_clock_skew', 120); } diff --git a/t/js_saml.t b/t/js_saml.t index 35119d0..b016a41 100644 --- a/t/js_saml.t +++ b/t/js_saml.t @@ -285,7 +285,7 @@ my $sp_pub = $t->read_file('sp.example.com.crt'); my $js_filename = 'saml_sp.js'; $t->write_file($js_filename, read_file("../$js_filename")); -$t->try_run('no njs available')->plan(132); +$t->try_run('no njs available')->plan(134); my $api_version = (sort { $a <=> $b } @{ api() })[-1]; my $kv = "/api/$api_version/http/keyvals"; @@ -393,7 +393,7 @@ like($r, qr{302.*http://sp.example.com:8080/foo\?a=b}s, like(get("$kv/saml_response_id"), qr/"_nginx_[^"]+":\s*"1"/, 'kv response id'); like(get("$kv/saml_name_id"), qr/user1/, 'kv response name id'); -like(get("$kv/saml_name_id_format"), qr/unspecified/, +like(get("$kv/saml_name_id_format"), qr/emailAddress/, 'kv response name id format'); like(get("$kv/saml_session_index"), qr/_nginx_sessionindex_/, 'kv response session index'); @@ -654,6 +654,8 @@ is($r->{Destination}, $cfg->{saml_idp_slo_url}, is($r->{Issuer}, $cfg->{saml_sp_entity_id}, 'sp logout request issuer'); is($r->{isSigned}, 0, 'sp logout request unsigned'); is($r->{NameID}, 'user1', 'sp logout request nameid'); +is($r->{NameIDFormat}, 'urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress', + 'sp logout request nameid format'); like(get("$kv/saml_request_id"), qr/"$r->{ID}":"1"/, 'sp logout request id redeemed'); @@ -764,6 +766,12 @@ $r = parse_response(modify_saml_obj($xml_obj, '//saml:NameID', 'text', 'foo', is($r->{StatusCode}, 'urn:oasis:names:tc:SAML:2.0:status:Requester', 'idp logout request wrong nameid'); +$r = parse_response(modify_saml_obj($xml_obj, '//saml:NameID', + 'Format', 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient', + auth_token => $auth_token)); +is($r->{StatusCode}, 'urn:oasis:names:tc:SAML:2.0:status:Requester', + 'idp logout request wrong nameid format'); + # Logout Response ($r, undef) = init_slo($cfg, relay_state => '/foo?a=b'); @@ -960,6 +968,11 @@ sub extract_saml_attributes { $result->{isSigned} = 0; } + my ($name_id_node) = $xpc->findnodes('//saml:NameID'); + if ($name_id_node) { + $result->{NameIDFormat} = $name_id_node->getAttribute('Format'); + } + my ($name_id_policy_node) = $xpc->findnodes('//samlp:NameIDPolicy'); if ($name_id_policy_node) { $result->{NameIDPolicyFormat} = @@ -1563,7 +1576,7 @@ END_XML $signature user1