fix(sso-oauth): feedback from review

The commit fixes some issues and feedback from
the review.

However, a new property has been introduced to
explicitly switch on/off the header authenticator.

Signed-off-by: Leo von Klenze <[email protected]>

rest/authorization-server/src/main/java/org/eclipse/sw360/rest/authserver/StringTransformer.java

@@ -0,0 +1,43 @@
+/*
+ * Copyright Siemens AG, 2019. Part of the SW360 Portal Project.
+ *
+ * SPDX-License-Identifier: EPL-1.0
+ *
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.eclipse.sw360.rest.authserver;
+
+public class StringTransformer {
+
+    /**
+     * Depending on the first parameter this method returns:
+     * <ul>
+     *   <li>null: null</li>
+     *   <li>String[] with at least one element: first element</li>
+     *   <li>String[] with no element: ""</li>
+     *   <li>other: string value of parameter</li>
+     * </ul>
+     * 
+     * @param object object to transform into a single string
+     * 
+     * @return the transformed string
+     */
+    public static String transformIntoString(Object object) {
+        if(object == null) {
+            return null;
+        }
+
+        if(object instanceof String[]) {
+            if(((String[]) object).length > 0) {
+                return ((String[])object)[0];
+            } else {
+                return "";
+            }
+        }
+
+        return object.toString();
+    }
+}

rest/authorization-server/src/main/java/org/eclipse/sw360/rest/authserver/client/rest/OAuthClientController.java

@@ -73,13 +73,15 @@ public ResponseEntity<?> createOrUpdateClient(@RequestBody OAuthClientResource c
             }
         } else {
             clientEntity = new OAuthClientEntity();
-            clientEntity.setId(UUID.randomUUID().toString().replace("-", ""));
+
+            // store entity to get a new id
+            repo.add(clientEntity);
+
             clientEntity.setClientId(clientEntity.getId());
             clientEntity.setClientSecret(UUID.randomUUID().toString());
         }
 
         updateClientEntityFromResource(clientEntity, clientResource);
-
         repo.update(clientEntity);
 
         return new ResponseEntity<OAuthClientResource>(

rest/authorization-server/src/main/java/org/eclipse/sw360/rest/authserver/security/Sw360AuthorizationServerConfiguration.java

@@ -80,12 +80,12 @@ public Sw360ClientDetailsService sw360ClientDetailsService() {
     @Bean
     public UserDetailsService userDetailsService() {
         return new Sw360UserDetailsService(sw360UserDetailsProvider, sw360ClientDetailsService(),
-                sw360UserAndClientAuthoritiesMerger());
+                sw360UserAndClientAuthoritiesCalculator());
     }
 
     @Bean
-    public Sw360UserAndClientAuthoritiesMerger sw360UserAndClientAuthoritiesMerger() {
-        return new Sw360UserAndClientAuthoritiesMerger();
+    public Sw360GrantedAuthoritiesCalculator sw360UserAndClientAuthoritiesCalculator() {
+        return new Sw360GrantedAuthoritiesCalculator();
     }
 
     @Bean

rest/authorization-server/src/main/java/org/eclipse/sw360/rest/authserver/security/Sw360UserAndClientAuthoritiesMerger.java → rest/authorization-server/src/main/java/org/eclipse/sw360/rest/authserver/security/Sw360GrantedAuthoritiesCalculator.java

@@ -25,20 +25,20 @@
 import static org.eclipse.sw360.rest.authserver.security.Sw360GrantedAuthority.READ;
 
 /**
- * Class only offers one single but very important method. It can calculate the
- * correct intersection between user and client authorities! Therefore it has to
+ * This class offer helper methods to calculate the {@GrantedAuthority} for a user and/or client. 
+ * In addition it can calculate the correct intersection between them! Therefore it has to
  * know how to map the sw360 user groups on rest authorities. This logic is also
  * centralized here implicitly.
  */
-public class Sw360UserAndClientAuthoritiesMerger {
+public class Sw360GrantedAuthoritiesCalculator {
 
     private final Logger log = Logger.getLogger(this.getClass());
 
-    public List<GrantedAuthority> mergeAuthoritiesOf(User user, ClientDetails clientDetails) {
+    public List<GrantedAuthority> generateFromUser(User user) {
         List<GrantedAuthority> grantedAuthorities = new ArrayList<>();
+
         grantedAuthorities.add(new SimpleGrantedAuthority(READ.getAuthority()));
-
-        if (!Objects.isNull(user)) {
+        if(user != null) {
             if (PermissionUtils.isUserAtLeast(Sw360AuthorizationServer.CONFIG_WRITE_ACCESS_USERGROUP, user)) {
                 grantedAuthorities.add(new SimpleGrantedAuthority(Sw360GrantedAuthority.WRITE.getAuthority()));
             }
@@ -47,16 +47,27 @@ public List<GrantedAuthority> mergeAuthoritiesOf(User user, ClientDetails client
             }
         }
 
-        if (!Objects.isNull(clientDetails)) {
-            Set<String> clientScopes = clientDetails.getScope();
+        return grantedAuthorities;
+    }
 
-            log.debug("User " + user.email + " has authorities " + grantedAuthorities + " while used client "
-                    + clientDetails.getClientId() + " has scopes " + clientScopes
-                    + ". Setting intersection as granted authorities for access token!");
+    public List<GrantedAuthority> intersectWithClient(List<GrantedAuthority> grantedAuthorities, ClientDetails clientDetails) {
+        Set<String> clientScopes = clientDetails.getScope();
 
-            grantedAuthorities = grantedAuthorities.stream()
-                    .filter(ga -> clientScopes.contains(ga.toString()))
-                    .collect(Collectors.toList());
+        grantedAuthorities = grantedAuthorities.stream()
+                .filter(ga -> clientScopes.contains(ga.toString()))
+                .collect(Collectors.toList());
+
+        return grantedAuthorities;
+    }
+
+    public List<GrantedAuthority> mergedAuthoritiesOf(User user, ClientDetails clientDetails) {
+        List<GrantedAuthority> grantedAuthorities = generateFromUser(user);
+
+        if(clientDetails != null) {
+            log.debug("User " + user.email + " has authorities " + grantedAuthorities + " while used client "
+                        + clientDetails.getClientId() + " has scopes " + clientDetails.getScope()
+                        + ". Setting intersection as granted authorities for access token!");
+            grantedAuthorities = intersectWithClient(grantedAuthorities, clientDetails);
         }
 
         return grantedAuthorities;

rest/authorization-server/src/main/java/org/eclipse/sw360/rest/authserver/security/Sw360UserDetailsService.java

@@ -36,13 +36,13 @@ public class Sw360UserDetailsService implements UserDetailsService {
 
     private Sw360ClientDetailsService clientProvider;
 
-    private Sw360UserAndClientAuthoritiesMerger authoritiesMerger;
+    private Sw360GrantedAuthoritiesCalculator authoritiesCalculator;
 
     public Sw360UserDetailsService(Sw360UserDetailsProvider userProvider, Sw360ClientDetailsService clientProvider,
-            Sw360UserAndClientAuthoritiesMerger authoritiesMerger) {
+            Sw360GrantedAuthoritiesCalculator authoritiesMerger) {
         this.userProvider = userProvider;
         this.clientProvider = clientProvider;
-        this.authoritiesMerger = authoritiesMerger;
+        this.authoritiesCalculator = authoritiesMerger;
     }
 
     @Override
@@ -63,7 +63,7 @@ public UserDetails loadUserByUsername(String username) throws UsernameNotFoundEx
 
                 if (clientDetails != null && user != null) {
                     result = new org.springframework.security.core.userdetails.User(user.getEmail(),
-                            "PreAuthenticatedPassword", authoritiesMerger.mergeAuthoritiesOf(user, clientDetails));
+                            "PreAuthenticatedPassword", authoritiesCalculator.mergedAuthoritiesOf(user, clientDetails));
                 }
             } catch (ClientRegistrationException e) {
                 log.warn("No valid client for id " + clientId + " could be found. It is possible that it is "

rest/authorization-server/src/main/java/org/eclipse/sw360/rest/authserver/security/basicauth/Sw360LiferayAuthenticationProvider.java

@@ -11,7 +11,7 @@
 package org.eclipse.sw360.rest.authserver.security.basicauth;
 
 import org.eclipse.sw360.datahandler.thrift.users.User;
-import org.eclipse.sw360.rest.authserver.security.Sw360UserAndClientAuthoritiesMerger;
+import org.eclipse.sw360.rest.authserver.security.Sw360GrantedAuthoritiesCalculator;
 import org.eclipse.sw360.rest.authserver.security.Sw360UserDetailsProvider;
 
 import org.apache.commons.lang.StringUtils;
@@ -43,7 +43,7 @@
  * In addition it supports the special password grant flow of spring in
  * retrieving information about the oauth client that has initiated the request
  * and cutting the user authorities to those of the client in such case by using
- * the {@link Sw360UserAndClientAuthoritiesMerger}.
+ * the {@link Sw360GrantedAuthoritiesCalculator}.
  */
 public class Sw360LiferayAuthenticationProvider implements AuthenticationProvider {
 
@@ -67,7 +67,7 @@ public class Sw360LiferayAuthenticationProvider implements AuthenticationProvide
     private Sw360UserDetailsProvider sw360CustomHeaderUserDetailsProvider;
 
     @Autowired
-    private Sw360UserAndClientAuthoritiesMerger sw360UserAndClientAuthoritiesMerger;
+    private Sw360GrantedAuthoritiesCalculator sw360UserAndClientAuthoritiesCalculator;
 
     @Override
     public Authentication authenticate(Authentication authentication) throws AuthenticationException {
@@ -85,7 +85,7 @@ public Authentication authenticate(Authentication authentication) throws Authent
                 if (!Objects.isNull(user)) {
                     ClientDetails clientDetails = extractClient(authentication);
                     return new UsernamePasswordAuthenticationToken(userIdentifier, password,
-                            sw360UserAndClientAuthoritiesMerger.mergeAuthoritiesOf(user, clientDetails));
+                            sw360UserAndClientAuthoritiesCalculator.mergedAuthoritiesOf(user, clientDetails));
                 }
             }
         }

rest/authorization-server/src/main/java/org/eclipse/sw360/rest/authserver/security/customheaderauth/Sw360CustomHeaderAuthenticationFilter.java

@@ -90,23 +90,39 @@ public class Sw360CustomHeaderAuthenticationFilter extends GenericFilterBean {
     @Value("${security.customheader.headername.intermediateauthstore:#{null}}")
     private String customHeaderHeadernameIntermediateAuthStore;
 
+    @Value("${security.customheader.headername.enabled:#{false}}")
+    private boolean customHeaderEnabled;
+
     private boolean active;
 
     @Autowired
     private Sw360UserDetailsProvider sw360CustomHeaderUserDetailsProvider;
 
     @PostConstruct
     public void postSw360CustomHeaderAuthenticationFilterConstruction() {
+        if(!customHeaderEnabled) {
+            active = false;
+            log.info("AuthenticationFilter is NOT active!");
+            return;
+        }
+
+        log.info("NOTE: Custom Header Authentication is enabled with the following configuration: \n" +
+            "  - email header      : " + customHeaderHeadernameEmail + "\n" +
+            "  - external id header: " + customHeaderHeadernameExtid + "\n" +
+            "  - internal header   : " + customHeaderHeadernameIntermediateAuthStore + "\n" +
+            "!!! BE SURE THAT THESE HEADRES ARE FILTERED BY YOUR PROXY! EACH CLIENT THAT IS ABLE TO SEND THESE HEADERS CAN LOG IN AS ANY PRINCIPAL !!!"
+        );
+
         if (StringUtils.isEmpty(customHeaderHeadernameEmail) || StringUtils.isEmpty(customHeaderHeadernameExtid)
                 || StringUtils.isEmpty(customHeaderHeadernameIntermediateAuthStore)) {
-            log.info("Filter is NOT active! If you want to activate it, please provide a complete configuration. "
+            log.info("AuthenticationFilter is NOT active due to incomplete configuration. "
                     + "Needed config keys:\n"
                     + "- security.customheader.headername.email\n"
                     + "- security.customheader.headername.extid\n"
                     + "- security.customheader.headername.intermediateauthstore");
             active = false;
         } else {
-            log.info("Filter is active!");
+            log.info("AuthenticationFilter is active!");
             active = true;
         }
     }