From 0c87ce603d7180117bfd80ec51eadf1166a4bcfd Mon Sep 17 00:00:00 2001 From: Jakub Sawicki Date: Fri, 15 Sep 2017 16:09:07 +0200 Subject: [PATCH] Correct thread-safe caching in CmdbOwnerService and several other fixes Removed IdentityProvider from places where it is unused. Corrected the way of handling provider.emails property. --- docker/indigo-slam.sh | 2 +- .../action/impl/HasAnyOfMetricsImpl.java | 7 +------ .../pl/cyfronet/ltos/controller/LegacyMethods.java | 9 --------- .../ltos/security/AuthenticationService.java | 10 ++++------ .../cyfronet/ltos/security/CmdbOwnerService.java | 12 +++--------- .../OpenIDConnectAuthenticationFilter.java | 14 ++++---------- .../pl/cyfronet/ltos/security/PortalUserImpl.java | 10 ++++------ 7 files changed, 17 insertions(+), 47 deletions(-) diff --git a/docker/indigo-slam.sh b/docker/indigo-slam.sh index 70de565..b20c3d8 100755 --- a/docker/indigo-slam.sh +++ b/docker/indigo-slam.sh @@ -16,7 +16,7 @@ java -Dserver.address=0.0.0.0 \ -Dunity.server.clientId=$IAM_CLIENT_ID \ -Dunity.server.clientSecret=$IAM_CLIENT_SECRET \ -Dcmdb.url=$CMDB_URL \ - -Dprovider.email=$PROVIDER_EMAIL \ + -Dprovider.emails=$PROVIDER_EMAIL \ -Djdbc.url=$JDBC_URL \ -Djdbc.username=$MYSQL_USER \ -Djdbc.password=$MYSQL_PASSWORD \ diff --git a/src/main/java/pl/cyfronet/bazaar/engine/extension/constraint/action/impl/HasAnyOfMetricsImpl.java b/src/main/java/pl/cyfronet/bazaar/engine/extension/constraint/action/impl/HasAnyOfMetricsImpl.java index 83f9391..9349f2b 100644 --- a/src/main/java/pl/cyfronet/bazaar/engine/extension/constraint/action/impl/HasAnyOfMetricsImpl.java +++ b/src/main/java/pl/cyfronet/bazaar/engine/extension/constraint/action/impl/HasAnyOfMetricsImpl.java @@ -1,11 +1,9 @@ package pl.cyfronet.bazaar.engine.extension.constraint.action.impl; import com.agreemount.bean.document.Document; -import com.agreemount.bean.identity.provider.IdentityProvider; import com.agreemount.slaneg.action.ActionContext; import com.agreemount.slaneg.constraint.action.impl.QualifierImpl; import com.google.common.base.Preconditions; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Scope; import org.springframework.stereotype.Component; import org.springframework.util.CollectionUtils; @@ -21,9 +19,6 @@ @Scope("prototype") public class HasAnyOfMetricsImpl extends QualifierImpl { - @Autowired - private IdentityProvider identityProvider; - @Override public boolean isAvailable() { String alias = getConstraintDefinition().getDocumentAlias(); @@ -46,4 +41,4 @@ public boolean isAvailable() { return result; } -} \ No newline at end of file +} diff --git a/src/main/java/pl/cyfronet/ltos/controller/LegacyMethods.java b/src/main/java/pl/cyfronet/ltos/controller/LegacyMethods.java index 49bd0e9..602bb88 100644 --- a/src/main/java/pl/cyfronet/ltos/controller/LegacyMethods.java +++ b/src/main/java/pl/cyfronet/ltos/controller/LegacyMethods.java @@ -1,8 +1,6 @@ package pl.cyfronet.ltos.controller; -import com.agreemount.bean.identity.provider.IdentityProvider; import lombok.extern.log4j.Log4j; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.security.core.context.SecurityContextHolder; @@ -12,7 +10,6 @@ import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.servlet.view.RedirectView; import pl.cyfronet.ltos.bean.User; -import pl.cyfronet.ltos.repository.UserRepository; import pl.cyfronet.ltos.security.UserInfo; import javax.servlet.http.HttpSession; @@ -22,12 +19,6 @@ @Log4j public class LegacyMethods { - @Autowired - private UserRepository userRepository; - - @Autowired - private IdentityProvider identityProvider; - @RequestMapping(value = "user/get", method = RequestMethod.GET) @Transactional public ResponseEntity getUser() throws IOException { diff --git a/src/main/java/pl/cyfronet/ltos/security/AuthenticationService.java b/src/main/java/pl/cyfronet/ltos/security/AuthenticationService.java index 1761893..530250b 100644 --- a/src/main/java/pl/cyfronet/ltos/security/AuthenticationService.java +++ b/src/main/java/pl/cyfronet/ltos/security/AuthenticationService.java @@ -9,12 +9,13 @@ import org.springframework.security.oauth2.client.OAuth2RestOperations; import org.springframework.stereotype.Service; import pl.cyfronet.ltos.bean.User; -import pl.cyfronet.ltos.repository.CmdbRepository; import pl.cyfronet.ltos.repository.UserRepository; import pl.cyfronet.ltos.security.AuthenticationProviderDev.UserOperations; import javax.net.ssl.HttpsURLConnection; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Locale; import java.util.stream.Collectors; @Service @@ -41,10 +42,7 @@ public class AuthenticationService { private UserOperations userOperations; @Autowired - IdentityProvider identityProvider; - - @Autowired - private CmdbRepository cmdbRepository; + private IdentityProvider identityProvider; @Autowired private PortalUserFactory portalUserFactory; diff --git a/src/main/java/pl/cyfronet/ltos/security/CmdbOwnerService.java b/src/main/java/pl/cyfronet/ltos/security/CmdbOwnerService.java index 3e26a08..fb6285a 100644 --- a/src/main/java/pl/cyfronet/ltos/security/CmdbOwnerService.java +++ b/src/main/java/pl/cyfronet/ltos/security/CmdbOwnerService.java @@ -31,20 +31,14 @@ public CmdbOwnerService( this.refreshInterval = refreshInterval; } - public Set getOwnedProviders(String email) { + public synchronized Set getOwnedProviders(String email) { if (isCacheInvalid()) { - synchronized (this) { - // If entering critical section after waiting on lock, the cache may already have - // been refreshed. So check again. - if (isCacheInvalid()) { - doRefresh(); - } - } + doRefresh(); } return ownerEmailToProviders.getOrDefault(email, Collections.EMPTY_SET); } - public void invalidateCache() { + public synchronized void invalidateCache() { lastRefresh = 0L; } diff --git a/src/main/java/pl/cyfronet/ltos/security/OpenIDConnectAuthenticationFilter.java b/src/main/java/pl/cyfronet/ltos/security/OpenIDConnectAuthenticationFilter.java index 97d2fd3..17ad9e6 100644 --- a/src/main/java/pl/cyfronet/ltos/security/OpenIDConnectAuthenticationFilter.java +++ b/src/main/java/pl/cyfronet/ltos/security/OpenIDConnectAuthenticationFilter.java @@ -1,11 +1,5 @@ package pl.cyfronet.ltos.security; -import java.io.IOException; - -import javax.servlet.ServletException; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.security.core.Authentication; @@ -17,7 +11,10 @@ import org.springframework.security.web.authentication.AbstractAuthenticationProcessingFilter; import org.springframework.security.web.authentication.SavedRequestAwareAuthenticationSuccessHandler; -import com.agreemount.bean.identity.provider.IdentityProvider; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; /** * Created by km on 04.08.16. @@ -27,9 +24,6 @@ public class OpenIDConnectAuthenticationFilter extends AbstractAuthenticationPro @Value("${unity.unauthorizedAction}") private String unauthorizedAction; - @Autowired - IdentityProvider identityProvider; - @Autowired private AuthenticationService authenticationService; diff --git a/src/main/java/pl/cyfronet/ltos/security/PortalUserImpl.java b/src/main/java/pl/cyfronet/ltos/security/PortalUserImpl.java index b090e86..4049e06 100644 --- a/src/main/java/pl/cyfronet/ltos/security/PortalUserImpl.java +++ b/src/main/java/pl/cyfronet/ltos/security/PortalUserImpl.java @@ -35,19 +35,17 @@ public static class Data { private final Data data; - private Set authorities = null; - @Autowired private CmdbOwnerService cmdbOwnerService; - //development variable, users whose email matches this will have provider role assigned - @Value("${provider.email:null}") - private Set devProviderEmails; - public PortalUserImpl(Data data) { this.data = data; } + /// development variable, users whose email matches this will have provider role assigned + @Value("${provider.emails:}") + private String devProviderEmails; + @Override public String getName() { return data.name;