saihemanth-cloudera commented on code in PR #4331:
URL: https://github.com/apache/hive/pull/4331#discussion_r1206089117


##########
service/src/java/org/apache/hive/service/auth/AuthType.java:
##########
@@ -18,55 +18,78 @@
 
 package org.apache.hive.service.auth;
 
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableSet;
+
 import org.apache.commons.lang3.EnumUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.conf.HiveServer2TransportMode;
 
-import java.util.Arrays;
+import java.util.ArrayList;
 import java.util.BitSet;
 import java.util.Collection;
-import java.util.HashSet;
 import java.util.Iterator;
+import java.util.List;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 /**
  * AuthType is used to parse and verify
  * {@link 
org.apache.hadoop.hive.conf.HiveConf.ConfVars#HIVE_SERVER2_AUTHENTICATION}.
  * Throws an exception if the config value is not allowed.
  */
 public class AuthType {
-  static final Set<HiveAuthConstants.AuthTypes> PASSWORD_BASED_TYPES = new 
HashSet<>(Arrays.asList(
-      HiveAuthConstants.AuthTypes.LDAP, HiveAuthConstants.AuthTypes.CUSTOM, 
HiveAuthConstants.AuthTypes.PAM));
+  static final Set<HiveAuthConstants.AuthTypes> PASSWORD_BASED_TYPES = 
ImmutableSet.of(HiveAuthConstants.AuthTypes.LDAP,
+      HiveAuthConstants.AuthTypes.CUSTOM, HiveAuthConstants.AuthTypes.PAM, 
HiveAuthConstants.AuthTypes.NONE);
   private final BitSet typeBits;
+  private final List<HiveAuthConstants.AuthTypes> authTypes;
+  private final HiveServer2TransportMode mode;
 
-  public AuthType(String authTypes) throws Exception {
+  @VisibleForTesting
+  public AuthType(String authTypes, HiveServer2TransportMode mode) {
+    this.authTypes = new ArrayList<>();
+    this.mode = mode;
     typeBits = new BitSet();
     parseTypes(authTypes);
     verifyTypes(authTypes);
   }
 
-  private void parseTypes(String authTypes) throws Exception {
+  private void parseTypes(String authTypes) {
     String[] types = authTypes.split(",");
     for (String type : types) {
       if (!EnumUtils.isValidEnumIgnoreCase(HiveAuthConstants.AuthTypes.class, 
type)) {
-        throw new Exception(type + " is not a valid authentication type.");
+        throw new IllegalArgumentException(type + " is not a valid 
authentication type.");
       }
-      
typeBits.set(EnumUtils.getEnumIgnoreCase(HiveAuthConstants.AuthTypes.class, 
type).ordinal());
+      HiveAuthConstants.AuthTypes authType = 
EnumUtils.getEnumIgnoreCase(HiveAuthConstants.AuthTypes.class, type);
+      this.authTypes.add(authType);
+      typeBits.set(authType.ordinal());
     }
   }
 
-  private void verifyTypes(String authTypes) throws Exception {
+  private void verifyTypes(String authTypes) {
     if (typeBits.cardinality() == 1) {
       // single authentication type has no conflicts
       return;
     }
-    if ((typeBits.get(HiveAuthConstants.AuthTypes.SAML.ordinal()) || 
typeBits.get(HiveAuthConstants.AuthTypes.JWT.ordinal())) &&
-        !typeBits.get(HiveAuthConstants.AuthTypes.NOSASL.ordinal()) &&
-        !typeBits.get(HiveAuthConstants.AuthTypes.KERBEROS.ordinal()) &&
-        !typeBits.get(HiveAuthConstants.AuthTypes.NONE.ordinal()) &&
-        (!areAnyEnabled(PASSWORD_BASED_TYPES) || 
isExactlyOneEnabled(PASSWORD_BASED_TYPES))) {
-      // SAML can be enabled with another password based authentication types
-      return;
+    if (typeBits.get(HiveAuthConstants.AuthTypes.NOSASL.ordinal())) {
+      throw new UnsupportedOperationException("NOSASL can't be along with 
other auth methods: " + authTypes);
+    }
+
+    if (typeBits.get(HiveAuthConstants.AuthTypes.NONE.ordinal())) {
+      throw new UnsupportedOperationException("None can't be along with other 
auth methods: " + authTypes);
+    }
+
+    if (areAnyEnabled(PASSWORD_BASED_TYPES) && 
!isExactlyOneEnabled(PASSWORD_BASED_TYPES)) {

Review Comment:
   Shouldn't we have multiple password-based types at once?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to