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

Auth Interceptor - make sure Id has a type before trying to get its associated resource #762

Merged
merged 1 commit into from Nov 23, 2017

Conversation

Projects
None yet
2 participants
@dconlan
Contributor

dconlan commented Oct 17, 2017

I have an issue with the authorization rules engine where I would get a null pointer exception during rule validation where I have a transaction with multiple inserts using temporary keys to link them.

This pull request is the one line fix which seems to solve the issue.

jamesagnew added a commit that referenced this pull request Nov 23, 2017

@jamesagnew

This comment has been minimized.

Show comment
Hide comment
@jamesagnew

jamesagnew Nov 23, 2017

Owner

Hi @dconlan - I'm always fairly cautious about changes to AuthorizationInterceptor since it's fairly critical.

I'm trying to reproduce the issue you are fixing and I'm not able to. Would you mind to have a look at the added test here to see if it's missing something? (It currently passes)

Owner

jamesagnew commented Nov 23, 2017

Hi @dconlan - I'm always fairly cautious about changes to AuthorizationInterceptor since it's fairly critical.

I'm trying to reproduce the issue you are fixing and I'm not able to. Would you mind to have a look at the added test here to see if it's missing something? (It currently passes)

@dconlan

This comment has been minimized.

Show comment
Hide comment
@dconlan

dconlan Nov 23, 2017

Contributor

Hi @jamesagnew

Must just be the weird combination of things I am using...

This test case exhibits the issue for me:

@Test
public void testInstanceRuleOkForResourceWithNoId() {
	
			ourRestServer.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) {
				@Override
				public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
					return new RuleBuilder()
						.allow("transactions").transaction().withAnyOperation().andApplyNormalRules().andThen()
						.allow("write patient").write().resourcesOfType(Patient.class).withAnyId().andThen()
						.allow("write encounter").write().resourcesOfType(Encounter.class).withAnyId().andThen()
						.allow("write condition").write().resourcesOfType(Condition.class).withAnyId().andThen()
						.denyAll("deny all")
						.build();
				}
			});	
	
	
			
			// Create a bundle that will be used as a transaction
			Bundle bundle = new Bundle();
			bundle.setType(Bundle.BundleType.TRANSACTION);
						
			
			
			String encounterId = "123-123";
			String encounterSystem = "http://our.internal.code.system/encounter";
			Encounter encounter = new Encounter();
			
			encounter.addIdentifier(new Identifier().setValue(encounterId)
								.setSystem(encounterSystem));
						
			encounter.setStatus(Encounter.EncounterStatus.FINISHED);
						
			Patient p = new Patient()
								.addIdentifier(new Identifier().setValue("321-321").setSystem("http://our.internal.code.system/patient"));
			p.setId(IdDt.newRandomUuid());
					
			// add patient to bundle so its created
			bundle.addEntry()
			   .setFullUrl(p.getId())
			   .setResource(p)
			   .getRequest()
  		      .setUrl("Patient")
		      .setMethod(Bundle.HTTPVerb.POST);
					
			Reference patientRef = new Reference(p.getId());
						
			encounter.setSubject(patientRef);
			Condition condition = new Condition()
							.setCode(new CodeableConcept().addCoding(
										new Coding("http://hl7.org/fhir/icd-10", "S53.40", "FOREARM SPRAIN / STRAIN")))
								.setSubject(patientRef);
								
			condition.setId(IdDt.newRandomUuid());
					
			// add condition to bundle so its created
			bundle.addEntry()
			   .setFullUrl(condition.getId())
			   .setResource(condition)
			   .getRequest()
			   .setUrl("Condition")
			   .setMethod(Bundle.HTTPVerb.POST);
					
			DiagnosisComponent dc = new DiagnosisComponent();
					
			dc.setCondition(new Reference(condition.getId()));
			encounter.addDiagnosis(dc);		
			CodeableConcept reason = new CodeableConcept();
			reason.setText("SLIPPED ON FLOOR,PAIN L) ELBOW");
			encounter.addReason(reason);
						
			// add encounter to bundle so its created
			bundle.addEntry()
			   .setResource(encounter)
			   .getRequest()
			      .setUrl("Encounter")
			      .setIfNoneExist("identifier=" + encounterSystem + "|" + encounterId)
			      .setMethod(Bundle.HTTPVerb.POST);

			
			Bundle resp = ourClient.transaction().withBundle(bundle).execute();
			assertEquals(3, resp.getEntry().size());
			
		}
Contributor

dconlan commented Nov 23, 2017

Hi @jamesagnew

Must just be the weird combination of things I am using...

This test case exhibits the issue for me:

@Test
public void testInstanceRuleOkForResourceWithNoId() {
	
			ourRestServer.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) {
				@Override
				public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
					return new RuleBuilder()
						.allow("transactions").transaction().withAnyOperation().andApplyNormalRules().andThen()
						.allow("write patient").write().resourcesOfType(Patient.class).withAnyId().andThen()
						.allow("write encounter").write().resourcesOfType(Encounter.class).withAnyId().andThen()
						.allow("write condition").write().resourcesOfType(Condition.class).withAnyId().andThen()
						.denyAll("deny all")
						.build();
				}
			});	
	
	
			
			// Create a bundle that will be used as a transaction
			Bundle bundle = new Bundle();
			bundle.setType(Bundle.BundleType.TRANSACTION);
						
			
			
			String encounterId = "123-123";
			String encounterSystem = "http://our.internal.code.system/encounter";
			Encounter encounter = new Encounter();
			
			encounter.addIdentifier(new Identifier().setValue(encounterId)
								.setSystem(encounterSystem));
						
			encounter.setStatus(Encounter.EncounterStatus.FINISHED);
						
			Patient p = new Patient()
								.addIdentifier(new Identifier().setValue("321-321").setSystem("http://our.internal.code.system/patient"));
			p.setId(IdDt.newRandomUuid());
					
			// add patient to bundle so its created
			bundle.addEntry()
			   .setFullUrl(p.getId())
			   .setResource(p)
			   .getRequest()
  		      .setUrl("Patient")
		      .setMethod(Bundle.HTTPVerb.POST);
					
			Reference patientRef = new Reference(p.getId());
						
			encounter.setSubject(patientRef);
			Condition condition = new Condition()
							.setCode(new CodeableConcept().addCoding(
										new Coding("http://hl7.org/fhir/icd-10", "S53.40", "FOREARM SPRAIN / STRAIN")))
								.setSubject(patientRef);
								
			condition.setId(IdDt.newRandomUuid());
					
			// add condition to bundle so its created
			bundle.addEntry()
			   .setFullUrl(condition.getId())
			   .setResource(condition)
			   .getRequest()
			   .setUrl("Condition")
			   .setMethod(Bundle.HTTPVerb.POST);
					
			DiagnosisComponent dc = new DiagnosisComponent();
					
			dc.setCondition(new Reference(condition.getId()));
			encounter.addDiagnosis(dc);		
			CodeableConcept reason = new CodeableConcept();
			reason.setText("SLIPPED ON FLOOR,PAIN L) ELBOW");
			encounter.addReason(reason);
						
			// add encounter to bundle so its created
			bundle.addEntry()
			   .setResource(encounter)
			   .getRequest()
			      .setUrl("Encounter")
			      .setIfNoneExist("identifier=" + encounterSystem + "|" + encounterId)
			      .setMethod(Bundle.HTTPVerb.POST);

			
			Bundle resp = ourClient.transaction().withBundle(bundle).execute();
			assertEquals(3, resp.getEntry().size());
			
		}
@jamesagnew

This comment has been minimized.

Show comment
Hide comment
@jamesagnew

jamesagnew Nov 23, 2017

Owner

Great! I have added that test to the codebase. I see the problem with my test, I was using a rule on a specific ID, not an "any ID" like yours.

Merging now!

Owner

jamesagnew commented Nov 23, 2017

Great! I have added that test to the codebase. I see the problem with my test, I was using a rule on a specific ID, not an "any ID" like yours.

Merging now!

@jamesagnew jamesagnew merged commit 193edea into jamesagnew:master Nov 23, 2017

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details

jamesagnew added a commit that referenced this pull request Nov 23, 2017

jamesagnew added a commit that referenced this pull request Nov 23, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment