fix: address PR review feedback from RegistrationGuard SPI (#271)

- Include denial reason in OAuth2Error description field so
  AuthenticationFailureHandlers can access it via getError().getDescription()
  in both DSOAuth2UserService and DSOidcUserService
- Replace String.trim().isEmpty() with String.isBlank() in
  RegistrationDecision.deny() (Java 17+ idiomatic)
- Extract DEFAULT_DENIAL_REASON constant in RegistrationDecision
- Add @FunctionalInterface to RegistrationGuard interface
- Add JavaDoc clarifying RegistrationDecision reason is only meaningful
  when allowed is false
- Add RegistrationContextTest covering null source validation

Author: devondragon

src/main/java/com/digitalsanctuary/spring/user/registration/RegistrationDecision.java

@@ -4,11 +4,19 @@
  * The result of a {@link RegistrationGuard} evaluation indicating whether
  * a registration attempt should be allowed or denied.
  *
+ * <p>Use the static factory methods {@link #allow()} and {@link #deny(String)}
+ * rather than the canonical constructor. The {@code reason} parameter is only
+ * meaningful when {@code allowed} is {@code false}.</p>
+ *
  * @param allowed whether the registration is permitted
- * @param reason  a human-readable denial reason (may be {@code null} when allowed)
+ * @param reason  a human-readable denial reason; only meaningful when {@code allowed}
+ *                is {@code false}, may be {@code null} when allowed
  */
 public record RegistrationDecision(boolean allowed, String reason) {
 
+    /** Default reason used when {@link #deny(String)} is called with a blank or null reason. */
+    private static final String DEFAULT_DENIAL_REASON = "Registration denied.";
+
     /**
      * Creates a decision that allows the registration to proceed.
      *
@@ -25,8 +33,8 @@ public static RegistrationDecision allow() {
      * @return a denying decision with the given reason
      */
     public static RegistrationDecision deny(String reason) {
-        if (reason == null || reason.trim().isEmpty()) {
-            reason = "Registration denied.";
+        if (reason == null || reason.isBlank()) {
+            reason = DEFAULT_DENIAL_REASON;
         }
         return new RegistrationDecision(false, reason);
     }

src/main/java/com/digitalsanctuary/spring/user/registration/RegistrationGuard.java

@@ -40,6 +40,7 @@
  * @see RegistrationDecision
  * @see DefaultRegistrationGuard
  */
+@FunctionalInterface
 public interface RegistrationGuard {
 
     /**

src/main/java/com/digitalsanctuary/spring/user/service/DSOAuth2UserService.java

@@ -110,7 +110,7 @@ public User handleOAuthLoginSuccess(String registrationId, OAuth2User oAuth2User
                 log.info("Registration denied for email: {} source: OAUTH2 provider: {} reason: {}",
                         user.getEmail(), registrationId, decision.reason());
                 throw new OAuth2AuthenticationException(
-                        new OAuth2Error("registration_denied"), decision.reason());
+                        new OAuth2Error("registration_denied", decision.reason(), null), decision.reason());
             }
             user = registerNewOAuthUser(registrationId, user);
             return user;

src/main/java/com/digitalsanctuary/spring/user/service/DSOidcUserService.java

@@ -109,7 +109,7 @@ public User handleOidcLoginSuccess(String registrationId, OidcUser oidcUser) {
                 log.info("Registration denied for email: {} source: OIDC provider: {} reason: {}",
                         user.getEmail(), registrationId, decision.reason());
                 throw new OAuth2AuthenticationException(
-                        new OAuth2Error("registration_denied"), decision.reason());
+                        new OAuth2Error("registration_denied", decision.reason(), null), decision.reason());
             }
             user = registerNewOidcUser(registrationId, user);
             return user;

src/test/java/com/digitalsanctuary/spring/user/registration/RegistrationContextTest.java

@@ -0,0 +1,39 @@
+package com.digitalsanctuary.spring.user.registration;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+@DisplayName("RegistrationContext Tests")
+class RegistrationContextTest {
+
+    @Test
+    @DisplayName("Should reject null source in RegistrationContext")
+    void shouldRejectNullSource() {
+        assertThatThrownBy(() -> new RegistrationContext("user@example.com", null, null))
+                .isInstanceOf(NullPointerException.class)
+                .hasMessageContaining("source must not be null");
+    }
+
+    @Test
+    @DisplayName("Should accept valid context with all fields")
+    void shouldAcceptValidContext() {
+        RegistrationContext context = new RegistrationContext("user@example.com", RegistrationSource.OAUTH2, "google");
+
+        assertThat(context.email()).isEqualTo("user@example.com");
+        assertThat(context.source()).isEqualTo(RegistrationSource.OAUTH2);
+        assertThat(context.providerName()).isEqualTo("google");
+    }
+
+    @Test
+    @DisplayName("Should accept null email and null providerName")
+    void shouldAcceptNullEmailAndProvider() {
+        RegistrationContext context = new RegistrationContext(null, RegistrationSource.FORM, null);
+
+        assertThat(context.email()).isNull();
+        assertThat(context.source()).isEqualTo(RegistrationSource.FORM);
+        assertThat(context.providerName()).isNull();
+    }
+}