From e70c5586ab3c75e76ea174e3cf02c606b7a1c1d4 Mon Sep 17 00:00:00 2001 From: Todd Kazakov Date: Tue, 29 Oct 2024 15:35:46 +0200 Subject: [PATCH] [BACK-3125] Address SMART IDP code review feedback and add small improvements --- .../broker/SMARTIdentityProvider.java | 51 ++++++++----------- 1 file changed, 20 insertions(+), 31 deletions(-) diff --git a/admin/src/main/java/org/tidepool/keycloak/extensions/broker/SMARTIdentityProvider.java b/admin/src/main/java/org/tidepool/keycloak/extensions/broker/SMARTIdentityProvider.java index c11e17b..2d10db9 100644 --- a/admin/src/main/java/org/tidepool/keycloak/extensions/broker/SMARTIdentityProvider.java +++ b/admin/src/main/java/org/tidepool/keycloak/extensions/broker/SMARTIdentityProvider.java @@ -16,8 +16,10 @@ import org.keycloak.broker.provider.IdentityBrokerException; import org.keycloak.broker.provider.util.SimpleHttp; import org.keycloak.models.KeycloakSession; +import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.representations.AccessTokenResponse; import org.keycloak.representations.JsonWebToken; +import org.tidepool.keycloak.extensions.broker.mappers.PatientsUserSessionNote; import java.io.IOException; import java.util.Arrays; @@ -26,22 +28,23 @@ public class SMARTIdentityProvider extends OIDCIdentityProvider { private static final Logger LOG = Logger.getLogger(SMARTIdentityProvider.class); - private static final String[] defaultForwardParameters = {"launch", "aud", "iss"}; + public static final String FHIR_VERSION = "smart/fhir_version"; + public static final String FHIR_BASE_URL = "smart/fhir_base_url"; - public static final String FHIR_R4 = "R4"; + private static final String[] DEFAULT_FORWARD_PARAMETERS = {"launch", "aud", "iss"}; private final SMARTIdentityProviderConfig config; public SMARTIdentityProvider(KeycloakSession session, SMARTIdentityProviderConfig config) { super(session, discoverConfig(session, config.getIssuer())); + + this.config = config; getConfig().setClientId(config.getClientId()); getConfig().setClientSecret(config.getClientSecret()); getConfig().setDefaultScope(config.getScopes()); getConfig().setAlias(config.getAlias()); getConfig().setForwardParameters(withDefaultForwardParameters(config.getForwardParameters())); getConfig().setDisableUserInfoService(true); - - this.config = config; } @Override @@ -50,7 +53,7 @@ protected BrokeredIdentityContext extractIdentity(AccessTokenResponse tokenRespo Practitioner practitioner = getPractitioner(idToken.getSubject(), accessToken); for (ContactPoint c : practitioner.getTelecom()) { - if (c.getSystem() == ContactPoint.ContactPointSystem.EMAIL && c.getValue() != null && !c.getValue().isEmpty()) { + if (c.getSystem() == ContactPoint.ContactPointSystem.EMAIL && c.getValue() != null && !c.getValue().isBlank()) { identity.setEmail(c.getValue()); break; } @@ -59,47 +62,31 @@ protected BrokeredIdentityContext extractIdentity(AccessTokenResponse tokenRespo identity.setFirstName(practitioner.getNameFirstRep().getGivenAsSingleString()); identity.setLastName(practitioner.getNameFirstRep().getFamily()); + identity.getContextData().put(FHIR_VERSION, config.getFHIRVersion()); + identity.getContextData().put(FHIR_BASE_URL, config.getIssuer()); + return identity; } private Practitioner getPractitioner(String id, String accessToken) { - if (!FHIR_R4.equals(config.getFHIRVersion())) { - throw new IdentityBrokerException("Unsupported FHIR Version: " + config.getFHIRVersion()); - } - - FhirContext ctx = FHIRContext.getR4(); - IClientInterceptor authInterceptor = new BearerTokenAuthInterceptor(accessToken); - IGenericClient client = ctx.newRestfulGenericClient(config.getIssuer()); - client.registerInterceptor(authInterceptor); + IGenericClient client = FHIRContext.getFHIRClient(config.getFHIRVersion(), config.getIssuer(), accessToken); Practitioner practitioner = client.read().resource(Practitioner.class).withId(id).execute(); if (LOG.isTraceEnabled()) { - IParser parser = ctx.newJsonParser(); - String serialized = parser.encodeResourceToString(practitioner); - LOG.tracef("Retrieved practitioner resource: " + serialized); + IParser parser = FHIRContext.getR4().newJsonParser(); + LOG.tracef("Retrieved practitioner resource: %s", parser.encodeResourceToString(practitioner)); } return practitioner; } - private Patient getPatient(String id, String accessToken) { - if (!FHIR_R4.equals(config.getFHIRVersion())) { - throw new IdentityBrokerException("Unsupported FHIR Version: " + config.getFHIRVersion()); - } - - FhirContext ctx = FHIRContext.getR4(); - IClientInterceptor authInterceptor = new BearerTokenAuthInterceptor(accessToken); - IGenericClient client = ctx.newRestfulGenericClient(config.getIssuer()); - return client.read().resource(Patient.class).withId(id).execute(); - } - private static String withDefaultForwardParameters(String params){ if (params == null) { params = ""; } HashSet set = new HashSet<>(Arrays.asList(params.split(","))); - set.addAll(Arrays.asList(defaultForwardParameters)); + set.addAll(Arrays.asList(DEFAULT_FORWARD_PARAMETERS)); return String.join(",", set.stream().map(String::trim).toArray(String[]::new)); } @@ -121,11 +108,13 @@ private static OIDCIdentityProviderConfig discoverConfig(KeycloakSession session try { SimpleHttp.Response response = request.asResponse(); if (response.getStatus() != 200) { - String msg = "failed to invoke url [" + smartConfigurationUrl + "]"; + String detail = String.format("Unexpected response %d", response.getStatus()); String tmp = response.asString(); - if (tmp != null) msg = tmp; + if (tmp != null) detail = tmp; + + String msg = String.format("Failed to invoke url [%s]: %s", smartConfigurationUrl, detail) ; - throw new IdentityBrokerException("Failed to invoke url [" + smartConfigurationUrl + "]: " + msg); + throw new IdentityBrokerException(msg); } identityProviderConfig.setConfig(factory.parseConfig(session, response.asString()));