Skip to content

Commit

Permalink
fix null pointer crash when building opa input
Browse files Browse the repository at this point in the history
  • Loading branch information
Charles Daniels committed Sep 19, 2024
1 parent 5b5d3c9 commit 5c4db24
Show file tree
Hide file tree
Showing 3 changed files with 134 additions and 19 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## v0.0.6 (unreleased)

* Fixed a null pointer exception while constructing the input to OPA with some Authentication implementations.

## v0.0.5

* Add `OPAAuthorizationManager` constructor that accepts a path and a `ContextDataProvider`, but not an `OPAClient`.
Expand Down
53 changes: 34 additions & 19 deletions src/main/java/com/styra/opa/springboot/OPAAuthorizationManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,22 +175,39 @@ public void setReasonKey(String newReasonKey) {
this.reasonKey = newReasonKey;
}

/**
* If v is null, this function is a NO-OP, otherwise, it calls m.put(k, v).
*
* @return
*/
private void nullablePut(Map<String, Object> m, String k, Object v) {
if (v == null) {
return;
}

m.put(k, v);
}

private Map<String, Object> makeRequestInput(
Supplier<Authentication> authentication,
RequestAuthorizationContext object
) {
Object subjectId = authentication.get().getPrincipal();
Object subjectDetails = authentication.get().getDetails();
Collection<? extends GrantedAuthority> subjectAuthorities =
authentication.get().getAuthorities();
Object subjectId = null;
Object subjectDetails = null;
Collection<? extends GrantedAuthority> subjectAuthorities = null;
if (authentication.get() != null) {
subjectId = authentication.get().getPrincipal();
subjectDetails = authentication.get().getDetails();
subjectAuthorities = authentication.get().getAuthorities();
}

HttpServletRequest request = object.getRequest();
String resourceId = request.getServletPath();

String actionName = request.getMethod();
String actionProtocol = request.getProtocol();
Enumeration<String> headerNamesEnumeration = request.getHeaderNames();
HashMap<String, String> headers = new HashMap<String, String>();
Map<String, String> headers = new HashMap<String, String>();
while (headerNamesEnumeration.hasMoreElements()) {
String headerName = headerNamesEnumeration.nextElement();
String headerValue = request.getHeader(headerName);
Expand All @@ -204,27 +221,25 @@ private Map<String, Object> makeRequestInput(
String contextRemoteHost = request.getRemoteHost();
Integer contextRemotePort = request.getRemotePort();

HashMap<String, Object> ctx = new HashMap<String, Object>();
ctx.put("type", RequestContextType);
ctx.put("host", contextRemoteHost);
ctx.put("ip", contextRemoteAddr);
ctx.put("port", contextRemotePort);
Map<String, Object> ctx = new HashMap<String, Object>();
nullablePut(ctx, "type", RequestContextType);
nullablePut(ctx, "host", contextRemoteHost);
nullablePut(ctx, "ip", contextRemoteAddr);
nullablePut(ctx, "port", contextRemotePort);

Map<String, Object> subjectInfo = new HashMap<String, Object>();
nullablePut(subjectInfo, "type", SubjectType);
nullablePut(subjectInfo, "id", subjectId);
nullablePut(subjectInfo, "details", subjectDetails);
nullablePut(subjectInfo, "authorities", subjectAuthorities);

if (this.ctxProvider != null) {
Object contextData = this.ctxProvider.getContextData(authentication, object);
ctx.put("data", contextData);
}

java.util.Map<String, Object> iMap = java.util.Map.ofEntries(
entry(
"subject",
java.util.Map.ofEntries(
entry("type", SubjectType),
entry("id", subjectId),
entry("details", subjectDetails),
entry("authorities", subjectAuthorities)
)
),
entry("subject", subjectInfo),
entry(
"resource",
java.util.Map.ofEntries(
Expand Down
98 changes: 98 additions & 0 deletions src/test/java/com/styra/opa/springboot/OPAAuthorizationManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,30 @@ private Authentication createMockAuthentication() {
return mockAuth;
}

/**
* This is used to create a mock auth object where most of the fields are
* null, to suss out exceptions when optional fields are omitted.
*
* @return
*/
private Authentication createNullMockAuthentication() {
Authentication mockAuth = mock(Authentication.class);
when(mockAuth.getPrincipal()).thenReturn("testuser");
when(mockAuth.getCredentials()).thenReturn(null);

WebAuthenticationDetails details = new WebAuthenticationDetails(httpServletRequest);
when(mockAuth.getDetails()).thenReturn(null);

GrantedAuthority authority1 = new SimpleGrantedAuthority("ROLE_USER");
GrantedAuthority authority2 = new SimpleGrantedAuthority("ROLE_ADMIN");
Collection<? extends GrantedAuthority> authorities = Arrays.asList(authority1, authority2);
doReturn(null).when(mockAuth).getAuthorities();

when(mockAuth.isAuthenticated()).thenReturn(true);

return mockAuth;
}

// Convert the value to JSON and then retrieve the value at the specified
// path.
//
Expand Down Expand Up @@ -403,4 +427,78 @@ public void testOPAAuthorizationManagerEcho() {

}

@Test
public void testOPAAuthorizationManagerNullMetadata() {
// By reading back the input, we can make sure the OPA input has the
// right structure and content.

Map<String, Object> expectData = Map.ofEntries(
entry("action", Map.ofEntries(
entry("headers", Map.ofEntries(
entry("UnitTestHeader", "123abc")
)),
entry("name", "GET"),
entry("protocol", "HTTP/1.1")
)),
entry("context", Map.ofEntries(
entry("host", "example.com"),
entry("ip", "192.0.2.123"),
entry("port", 0),
entry("type", "http"),
entry("data", Map.ofEntries(
entry("hello", "world")
))
)),
entry("resource", Map.ofEntries(
entry("id", "unit/test"),
entry("type", "endpoint")
)),
entry("subject", Map.ofEntries(
entry("id", "testuser"),
entry("type", "java_authentication")
))
);

OPAResponseContext expectCtx = new OPAResponseContext();
expectCtx.setReasonUser(Map.ofEntries(
entry("en", "echo rule always allows"),
entry("other", "other reason key")
));
expectCtx.setId("0");
expectCtx.setData(expectData);

OPAResponse expect = new OPAResponse();
expect.setDecision(true);
expect.setContext(expectCtx);

ContextDataProvider prov = new ConstantContextDataProvider(java.util.Map.ofEntries(
entry("hello", "world")
));
Authentication mockAuth = this.createNullMockAuthentication();
when(authenticationSupplier.get()).thenReturn(mockAuth);
OPAClient opa = new OPAClient(address, headers);
OPAAuthorizationManager am = new OPAAuthorizationManager(opa, "policy/echo", prov);
OPAResponse actual = am.opaRequest(this.authenticationSupplier, this.context);

assertEquals(expect.getDecision(), actual.getDecision());
assertEquals(expect.getContext().getId(), actual.getContext().getId());
assertEquals(expect.getContext().getReasonUser(), actual.getContext().getReasonUser());

List<String> datadiff = jsonDiff(expect.getContext().getData(), actual.getContext().getData());

System.out.printf("#### expected context data\n%s\n", jsonPretty(expectData));
System.out.printf("#### actual context data\n%s\n", jsonPretty(actual.getContext().getData()));

for (int i = 0; i < datadiff.size(); i++) {
System.out.printf("diff mismatch: %s\n", datadiff.get(i));
}

assertEquals(0, datadiff.size());

assertEquals("echo rule always allows", actual.getReasonForDecision("en"));
assertEquals("other reason key", actual.getReasonForDecision("other"));
assertEquals("echo rule always allows", actual.getReasonForDecision("nonexistant"));

}

}

0 comments on commit 5c4db24

Please sign in to comment.