From 0adb4aa6a5632504d006a3364118915201bae34d Mon Sep 17 00:00:00 2001 From: Nico Verwer Date: Mon, 9 Jan 2023 13:38:08 +0100 Subject: [PATCH 1/2] [bugfix] guard against NPE in securitymanager fixes #4670 --- .../functions/securitymanager/IdFunction.java | 15 ++++++++++----- .../functions/securitymanager/IdFunctionTest.java | 10 +++++----- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/exist-core/src/main/java/org/exist/xquery/functions/securitymanager/IdFunction.java b/exist-core/src/main/java/org/exist/xquery/functions/securitymanager/IdFunction.java index 3d156836aa8..3bcd25446f5 100644 --- a/exist-core/src/main/java/org/exist/xquery/functions/securitymanager/IdFunction.java +++ b/exist-core/src/main/java/org/exist/xquery/functions/securitymanager/IdFunction.java @@ -77,13 +77,18 @@ private org.exist.dom.memtree.DocumentImpl functionId() { builder.startElement(new QName("id", SecurityManagerModule.NAMESPACE_URI, SecurityManagerModule.PREFIX), null); - builder.startElement(new QName("real", SecurityManagerModule.NAMESPACE_URI, SecurityManagerModule.PREFIX), null); - subjectToXml(builder, context.getRealUser()); - builder.endElement(); + final Subject realUser = context.getRealUser(); + if (realUser != null) { + builder.startElement(new QName("real", SecurityManagerModule.NAMESPACE_URI, SecurityManagerModule.PREFIX), null); + subjectToXml(builder, realUser); + builder.endElement(); + } - if (!sameUserWithSameGroups(context.getRealUser(), context.getEffectiveUser())) { + final Subject effectiveUser = context.getEffectiveUser(); + if (effectiveUser != null && ( + realUser == null || !sameUserWithSameGroups(realUser, effectiveUser))) { builder.startElement(new QName("effective", SecurityManagerModule.NAMESPACE_URI, SecurityManagerModule.PREFIX), null); - subjectToXml(builder, context.getEffectiveUser()); + subjectToXml(builder, effectiveUser); builder.endElement(); } diff --git a/exist-core/src/test/java/org/exist/xquery/functions/securitymanager/IdFunctionTest.java b/exist-core/src/test/java/org/exist/xquery/functions/securitymanager/IdFunctionTest.java index 2558cef9f00..fa06db09808 100644 --- a/exist-core/src/test/java/org/exist/xquery/functions/securitymanager/IdFunctionTest.java +++ b/exist-core/src/test/java/org/exist/xquery/functions/securitymanager/IdFunctionTest.java @@ -71,14 +71,14 @@ public void differingRealAndEffectiveUsers() throws XPathException, XpathExcepti expect(mckContext.getDocumentBuilder()).andReturn(new MemTreeBuilder()); mckContext.popDocumentContext(); expectLastCall().once(); - expect(mckContext.getRealUser()).andReturn(mckRealUser).times(2); + expect(mckContext.getRealUser()).andReturn(mckRealUser); expect(mckRealUser.getName()).andReturn(realUsername); expect(mckRealUser.getGroups()).andReturn(new String[]{"realGroup1", "realGroup2"}); expect(mckRealUser.getId()).andReturn(1); final Subject mckEffectiveUser = EasyMock.createMock(Subject.class); final String effectiveUsername = "effective"; - expect(mckContext.getEffectiveUser()).andReturn(mckEffectiveUser).times(2); + expect(mckContext.getEffectiveUser()).andReturn(mckEffectiveUser); expect(mckEffectiveUser.getId()).andReturn(2); expect(mckEffectiveUser.getName()).andReturn(effectiveUsername); expect(mckEffectiveUser.getGroups()).andReturn(new String[]{"effectiveGroup1", "effectiveGroup2"}); @@ -127,7 +127,7 @@ public void sameRealAndEffectiveUsers() throws XPathException, XpathException { expect(mckContext.getDocumentBuilder()).andReturn(new MemTreeBuilder()); mckContext.popDocumentContext(); expectLastCall().once(); - expect(mckContext.getRealUser()).andReturn(mckUser).times(2); + expect(mckContext.getRealUser()).andReturn(mckUser); expect(mckUser.getName()).andReturn(username); expect(mckUser.getGroups()).andReturn(new String[]{"group1", "group2"}); expect(mckUser.getId()).andReturn(1); @@ -183,7 +183,7 @@ public void differingByGroupRealAndEffectiveUsers() throws XPathException, Xpath expect(mckContext.getDocumentBuilder()).andReturn(new MemTreeBuilder()); mckContext.popDocumentContext(); expectLastCall().once(); - expect(mckContext.getRealUser()).andReturn(mckRealUser).times(2); + expect(mckContext.getRealUser()).andReturn(mckRealUser); expect(mckRealUser.getName()).andReturn(realUsername); expect(mckRealUser.getGroups()).andReturn(new String[]{"realGroup1"}); expect(mckRealUser.getId()).andReturn(101); @@ -191,7 +191,7 @@ public void differingByGroupRealAndEffectiveUsers() throws XPathException, Xpath final Subject mckEffectiveUser = EasyMock.createMock(Subject.class); final String effectiveUsername = "user1"; - expect(mckContext.getEffectiveUser()).andReturn(mckEffectiveUser).times(2); + expect(mckContext.getEffectiveUser()).andReturn(mckEffectiveUser); expect(mckEffectiveUser.getId()).andReturn(101); expect(mckEffectiveUser.getName()).andReturn(effectiveUsername); expect(mckEffectiveUser.getGroups()).andReturn(new String[]{"realGroup1", "effectiveGroup1"}); From 5ee14e984857f2e10180fb1f51ce89c8959d2456 Mon Sep 17 00:00:00 2001 From: Juri Leino Date: Tue, 12 Dec 2023 12:45:48 +0100 Subject: [PATCH 2/2] [refactor] SecurityManager IdFunction Move null check into function sameUserWithSameGroups. --- .../xquery/functions/securitymanager/IdFunction.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/exist-core/src/main/java/org/exist/xquery/functions/securitymanager/IdFunction.java b/exist-core/src/main/java/org/exist/xquery/functions/securitymanager/IdFunction.java index 3bcd25446f5..613eaaf9063 100644 --- a/exist-core/src/main/java/org/exist/xquery/functions/securitymanager/IdFunction.java +++ b/exist-core/src/main/java/org/exist/xquery/functions/securitymanager/IdFunction.java @@ -33,6 +33,7 @@ import org.exist.xquery.value.Sequence; import org.exist.xquery.value.Type; +import javax.annotation.Nullable; import java.util.Arrays; /** @@ -85,8 +86,7 @@ private org.exist.dom.memtree.DocumentImpl functionId() { } final Subject effectiveUser = context.getEffectiveUser(); - if (effectiveUser != null && ( - realUser == null || !sameUserWithSameGroups(realUser, effectiveUser))) { + if (effectiveUser != null && !sameUserWithSameGroups(realUser, effectiveUser)) { builder.startElement(new QName("effective", SecurityManagerModule.NAMESPACE_URI, SecurityManagerModule.PREFIX), null); subjectToXml(builder, effectiveUser); builder.endElement(); @@ -102,7 +102,10 @@ private org.exist.dom.memtree.DocumentImpl functionId() { } } - private static boolean sameUserWithSameGroups(final Subject user1, final Subject user2) { + private static boolean sameUserWithSameGroups(@Nullable final Subject user1, @Nullable final Subject user2) { + if (user1 == null || user2 == null) { + return false; + } if (user1.getId() != user2.getId()) { return false; }