Responding to more PR comments

civiform · Mar 22, 2022 · 704187b · 704187b
1 parent f3457f9
commit 704187b
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 24 deletions.
diff --git a/universal-application-tool-0.0.1/app/auth/AdminAuthClient.java b/universal-application-tool-0.0.1/app/auth/AdminAuthClient.java
@@ -6,7 +6,7 @@
 
 /**
  * AdminAuthClient is the annotation for the auth client responsible for admin authentication. This
- * client must implement IndirectClient -> {@link org.pac4j.core.client.IndirectClient}.
+ * client must implement {@link org.pac4j.core.client.IndirectClient}.
  */
 @Qualifier
 @Retention(RetentionPolicy.RUNTIME)

diff --git a/universal-application-tool-0.0.1/app/auth/saml/LoginRadiusSamlProvider.java b/universal-application-tool-0.0.1/app/auth/saml/LoginRadiusSamlProvider.java
@@ -5,6 +5,7 @@
 import auth.ProfileFactory;
 import com.google.inject.Inject;
 import com.typesafe.config.Config;
+import com.typesafe.config.ConfigException;
 import java.util.IllegalFormatException;
 import java.util.Optional;
 import javax.inject.Provider;
@@ -69,16 +70,18 @@ public SAML2Client get() {
   }
 
   private Optional<String> formatMetadataResourceUrl() {
-    String metadataResourceUrl;
     try {
-      metadataResourceUrl =
+      String metadataResourceUrl =
           String.format(
               "%s?apikey=%s&appName=%s",
               configuration.getString("login_radius.metadata_uri"),
               configuration.getString("login_radius.api_key"),
               configuration.getString("login_radius.saml_app_name"));
       return Optional.of(metadataResourceUrl);
-    } catch (IllegalFormatException | NullPointerException e) {
+    } catch (IllegalFormatException
+        | NullPointerException
+        | ConfigException.Missing
+        | ConfigException.WrongType e) {
       return Optional.empty();
     }
   }

diff --git a/universal-application-tool-0.0.1/app/controllers/LoginController.java b/universal-application-tool-0.0.1/app/controllers/LoginController.java
@@ -73,27 +73,38 @@ public Result register(Http.Request request) {
       // Default to IDCS.
       idp = AuthIdentityProviderName.IDCS_APPLICANT.toString();
     }
-    // This register behavior is specific to IDCS. Because this is only being called when we know
-    // IDCS is available, it should technically never go into the second flow.
-    if (idp.equals(AuthIdentityProviderName.IDCS_APPLICANT.toString())) {
-      String registerUrl = null;
-      try {
-        registerUrl = config.getString("idcs.register_uri");
-      } catch (ConfigException.Missing e) {
-        // leave it as null / empty.
-      }
-      if (Strings.isNullOrEmpty(registerUrl)) {
-        return badRequest("Registration is not enabled.");
-      }
-      // Redirect to the registration URL - then, when the user visits the site again, automatically
-      // log them in.
-      return redirect(registerUrl)
-          .addingToSession(
-              request,
-              REDIRECT_TO_SESSION_KEY,
-              routes.LoginController.applicantLogin(Optional.empty()).url());
+
+    boolean isIDCS = idp.equals(AuthIdentityProviderName.IDCS_APPLICANT.toString());
+
+    // Because this is only being called when we know IDCS is available, this route should
+    // technically
+    // never happen.
+    if (!isIDCS) {
+      return login(request, applicantClient);
+    }
+
+    return idcsRegister(request);
+  }
+
+  // IDCS has specific register behavior that is different from other IDPs, which have the register
+  // option on the same screen as the login page.
+  private Result idcsRegister(Http.Request request) {
+    String registerUrl = null;
+    try {
+      registerUrl = config.getString("idcs.register_uri");
+    } catch (ConfigException.Missing e) {
+      // leave it as null / empty.
+    }
+    if (Strings.isNullOrEmpty(registerUrl)) {
+      return badRequest("Registration is not enabled.");
     }
-    return login(request, applicantClient);
+    // Redirect to the registration URL - then, when the user visits the site again, automatically
+    // log them in.
+    return redirect(registerUrl)
+        .addingToSession(
+            request,
+            REDIRECT_TO_SESSION_KEY,
+            routes.LoginController.applicantLogin(Optional.empty()).url());
   }
 
   // Logic taken from org.pac4j.play.deadbolt2.Pac4jHandler.beforeAuthCheck.