[java-idp-oidc] 02/02: Unify the OIDC admin flow error response messages.

Henri Mikkonen henri.mikkonen at iki.fi
Mon Mar 28 14:33:01 UTC 2022


This is an automated email from the git hooks/post-receive script.

hjmikkon pushed a commit to branch main
in repository java-idp-oidc.

View the commit online:
http://git.shibboleth.net/view/?p=java-idp-oidc.git;a=commit;h=349dd2305df7211e0058062d09e149534ff6f8c6

commit 349dd2305df7211e0058062d09e149534ff6f8c6
Author: Henri Mikkonen <henri.mikkonen at iki.fi>
AuthorDate: Mon Mar 28 17:31:10 2022 +0300

    Unify the OIDC admin flow error response messages.
    
    Like admin/oidc/clients flow, use JSON structure for error
    messages in issue-registration-access-token flow instead of
    building events for the error view.
---
 .../admin/impl/AbstractAdminApiProfileAction.java  | 127 ++++++++++++++++++
 .../op/admin/impl/DoClientManagementOperation.java |  64 ++-------
 .../admin/impl/IssueRegistrationAccessToken.java   | 144 +++++++++++----------
 .../impl/IssueRegistrationAccessTokenTest.java     |  42 +++++-
 .../flow/IssueRegistrationAccessTokenFlowTest.java |  25 +++-
 5 files changed, 270 insertions(+), 132 deletions(-)

diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/admin/impl/AbstractAdminApiProfileAction.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/admin/impl/AbstractAdminApiProfileAction.java
new file mode 100644
index 00000000..7d9a3eda
--- /dev/null
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/admin/impl/AbstractAdminApiProfileAction.java
@@ -0,0 +1,127 @@
+/*
+ * Licensed to the University Corporation for Advanced Internet Development,
+ * Inc. (UCAID) under one or more contributor license agreements.  See the
+ * NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The UCAID licenses this file to You under the Apache
+ * License, Version 2.0 (the "License"); you may not use this file except in
+ * compliance with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.shibboleth.idp.plugin.oidc.op.admin.impl;
+
+import java.io.IOException;
+import java.util.Collections;
+
+import javax.annotation.Nonnull;
+import javax.servlet.http.HttpServletResponse;
+
+import org.opensaml.profile.action.ActionSupport;
+import org.opensaml.profile.action.EventIds;
+import org.opensaml.profile.context.ProfileRequestContext;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.github.jasminb.jsonapi.models.errors.Error;
+import com.github.jasminb.jsonapi.models.errors.Errors;
+
+import net.shibboleth.idp.profile.AbstractProfileAction;
+import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
+import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
+import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
+import net.shibboleth.utilities.java.support.component.ComponentSupport;
+import net.shibboleth.utilities.java.support.logic.Constraint;
+
+/**
+ * Base class for admin flow actions producing JSON responses.
+ *
+ * @event {@link EventIds#INVALID_PROFILE_CTX}
+ */
+public class AbstractAdminApiProfileAction extends AbstractProfileAction {
+
+    /** Class logger. */
+    @Nonnull private Logger log = LoggerFactory.getLogger(AbstractAdminApiProfileAction.class);
+
+    /** JSON object mapper. */
+    @NonnullAfterInit private ObjectMapper objectMapper;
+
+    /**
+     * Set the JSON {@link ObjectMapper} to use for serialization.
+     * 
+     * @param mapper object mapper
+     */
+    public void setObjectMapper(@Nonnull final ObjectMapper mapper) {
+        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        
+        objectMapper = Constraint.isNotNull(mapper, "ObjectMapper cannot be null");
+    }
+
+    /**
+     * Get the JSON {@link ObjectMapper} to use for serialization.
+     * 
+     * @return The JSON {@link ObjectMapper} to use for serialization.
+     */
+    @NonnullAfterInit public ObjectMapper getObjectMapper() {
+        return objectMapper;
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    protected void doInitialize() throws ComponentInitializationException {
+        super.doInitialize();
+        
+        if (objectMapper == null) {
+            throw new ComponentInitializationException("ObjectMapper cannot be null");
+        }
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    protected boolean doPreExecute(final ProfileRequestContext profileRequestContext) {
+        
+        if (!super.doPreExecute(profileRequestContext)) {
+            return false;
+        } else if (getHttpServletRequest() == null || getHttpServletResponse() == null) {
+            log.warn("{} No HttpServletRequest or HttpServletResponse available", getLogPrefix());
+            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
+            return false;
+        }
+        return true;
+    }
+
+    /**
+     * Output an error object.
+     * 
+     * @param status HTTP status
+     * @param title fixed error description
+     * @param detail human-readable error description
+     * 
+     * @throws IOException if unable to output the error
+     */
+    protected void sendError(final int status, @Nonnull @NotEmpty final String title,
+            @Nonnull @NotEmpty final String detail) throws IOException {
+        
+        final HttpServletResponse response = getHttpServletResponse();
+        response.setContentType("application/json");
+        response.setHeader("Cache-Control", "must-revalidate,no-cache,no-store");
+        response.setStatus(status);
+        
+        final Error e = new Error();
+        final Errors errors = new Errors();
+        errors.setErrors(Collections.singletonList(e));
+        e.setStatus(Integer.toString(status));
+        e.setTitle(title);
+        e.setDetail(detail);
+        
+        objectMapper.writer().withDefaultPrettyPrinter().writeValue(response.getOutputStream(), errors);
+    }
+
+}
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/admin/impl/DoClientManagementOperation.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/admin/impl/DoClientManagementOperation.java
index 57c832d7..4a9cd082 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/admin/impl/DoClientManagementOperation.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/admin/impl/DoClientManagementOperation.java
@@ -18,14 +18,12 @@
 package net.shibboleth.idp.plugin.oidc.op.admin.impl;
 
 import java.io.IOException;
-import java.util.Collections;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
-import net.shibboleth.idp.profile.AbstractProfileAction;
 import net.shibboleth.idp.profile.context.SpringRequestContext;
 import net.shibboleth.oidc.metadata.ClientInformationManager;
 import net.shibboleth.oidc.metadata.ClientInformationManagerException;
@@ -46,9 +44,6 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.webflow.execution.RequestContext;
 
-import com.fasterxml.jackson.databind.ObjectMapper;
-import com.github.jasminb.jsonapi.models.errors.Error;
-import com.github.jasminb.jsonapi.models.errors.Errors;
 import com.google.common.base.Strings;
 import com.nimbusds.oauth2.sdk.id.ClientID;
 import com.nimbusds.openid.connect.sdk.rp.OIDCClientInformation;
@@ -64,7 +59,7 @@ import com.nimbusds.openid.connect.sdk.rp.OIDCClientInformation;
  * 
  * @since 3.1.0
  */
-public class DoClientManagementOperation extends AbstractProfileAction {
+public class DoClientManagementOperation extends AbstractAdminApiProfileAction {
     
     /** Flow variable indicating ID of storage key. */
     @Nonnull @NotEmpty public static final String CLIENT_ID = "clientId";
@@ -72,9 +67,6 @@ public class DoClientManagementOperation extends AbstractProfileAction {
     /** Class logger. */
     @Nonnull private Logger log = LoggerFactory.getLogger(DoClientManagementOperation.class);
     
-    /** JSON object mapper. */
-    @NonnullAfterInit private ObjectMapper objectMapper;
-    
     /** {@link ClientInformationResolver} to operate on. */
     @NonnullAfterInit private ClientInformationResolver resolver;
 
@@ -84,17 +76,6 @@ public class DoClientManagementOperation extends AbstractProfileAction {
     /** Client ID to operate on. */
     @Nullable @NotEmpty private String clientId;
 
-    /**
-     * Set the JSON {@link ObjectMapper} to use for serialization.
-     * 
-     * @param mapper object mapper
-     */
-    public void setObjectMapper(@Nonnull final ObjectMapper mapper) {
-        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
-        
-        objectMapper = Constraint.isNotNull(mapper, "ObjectMapper cannot be null");
-    }
-
    /**
     * Set the {@link ClientInformationResolver} to use for retrieval.
     * 
@@ -121,11 +102,15 @@ public class DoClientManagementOperation extends AbstractProfileAction {
     @Override
     protected void doInitialize() throws ComponentInitializationException {
         super.doInitialize();
-        
-        if (objectMapper == null) {
-            throw new ComponentInitializationException("ObjectMapper cannot be null");
+
+        if (resolver == null) {
+            throw new ComponentInitializationException("ClientInformationResolver cannot be null");
         }
-    }
+
+        if (manager == null) {
+            throw new ComponentInitializationException("ClientInformationManager cannot be null");
+        }
+   }
 
     /** {@inheritDoc} */
     @Override
@@ -133,10 +118,6 @@ public class DoClientManagementOperation extends AbstractProfileAction {
         
         if (!super.doPreExecute(profileRequestContext)) {
             return false;
-        } else if (getHttpServletRequest() == null || getHttpServletResponse() == null) {
-            log.warn("{} No HttpServletRequest or HttpServletResponse available", getLogPrefix());
-            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
-            return false;
         }
         
         try {
@@ -222,31 +203,4 @@ public class DoClientManagementOperation extends AbstractProfileAction {
         }
     }
 
-    /**
-     * Output an error object.
-     * 
-     * @param status HTTP status
-     * @param title fixed error description
-     * @param detail human-readable error description
-     * 
-     * @throws IOException if unable to output the error
-     */
-    private void sendError(final int status, @Nonnull @NotEmpty final String title,
-            @Nonnull @NotEmpty final String detail) throws IOException {
-        
-        final HttpServletResponse response = getHttpServletResponse();
-        response.setContentType("application/json");
-        response.setHeader("Cache-Control", "must-revalidate,no-cache,no-store");
-        response.setStatus(status);
-        
-        final Error e = new Error();
-        final Errors errors = new Errors();
-        errors.setErrors(Collections.singletonList(e));
-        e.setStatus(Integer.toString(status));
-        e.setTitle(title);
-        e.setDetail(detail);
-        
-        objectMapper.writer().withDefaultPrettyPrinter().writeValue(response.getOutputStream(), errors);
-    }
-    
 }
\ No newline at end of file
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/admin/impl/IssueRegistrationAccessToken.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/admin/impl/IssueRegistrationAccessToken.java
index 409aff07..bb41f5ee 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/admin/impl/IssueRegistrationAccessToken.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/admin/impl/IssueRegistrationAccessToken.java
@@ -17,6 +17,7 @@
 
 package net.shibboleth.idp.plugin.oidc.op.admin.impl;
 
+import java.io.IOException;
 import java.time.Duration;
 import java.time.Instant;
 import java.time.format.DateTimeParseException;
@@ -25,6 +26,7 @@ import java.util.function.Function;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
+import javax.servlet.http.HttpServletResponse;
 
 import org.opensaml.messaging.context.MessageContext;
 import org.opensaml.profile.action.ActionSupport;
@@ -34,7 +36,6 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
-import com.fasterxml.jackson.databind.ObjectMapper;
 import com.nimbusds.oauth2.sdk.AccessTokenResponse;
 import com.nimbusds.oauth2.sdk.TokenResponse;
 import com.nimbusds.oauth2.sdk.token.AccessToken;
@@ -45,7 +46,6 @@ import net.shibboleth.idp.authn.context.AuthenticationContext;
 import net.shibboleth.idp.authn.context.SubjectContext;
 import net.shibboleth.idp.plugin.oidc.op.cli.IssueRegistrationAccessTokenArguments;
 import net.shibboleth.idp.plugin.oidc.op.token.support.RegistrationClaimsSet;
-import net.shibboleth.idp.profile.AbstractProfileAction;
 import net.shibboleth.idp.profile.context.navigate.ResponderIdLookupFunction;
 import net.shibboleth.idp.profile.function.SpringFlowScopeLookupFunction;
 import net.shibboleth.oidc.metadata.policy.MetadataPolicy;
@@ -71,14 +71,12 @@ import net.shibboleth.utilities.java.support.security.impl.SecureRandomIdentifie
  * <p>Several access control checks are made to named policies in the case that certain options are supplied.</p>
  * 
  * @event {@link EventIds#PROCEED_EVENT_ID}
- * @event {@link EventIds#ACCESS_DENIED}
- * @event {@link EventIds#INVALID_MESSAGE}
  * @event {@link EventIds#INVALID_PROFILE_CTX}
  * @event {@link EventIds#IO_ERROR}
  * 
  * @since 3.1.0
  */
-public class IssueRegistrationAccessToken extends AbstractProfileAction {
+public class IssueRegistrationAccessToken extends AbstractAdminApiProfileAction {
     
     /** Class logger. */
     @Nonnull private Logger log = LoggerFactory.getLogger(IssueRegistrationAccessToken.class);
@@ -89,9 +87,6 @@ public class IssueRegistrationAccessToken extends AbstractProfileAction {
     /** Strategy used to locate the {@link IdentifierGenerationStrategy} to use. */
     @Nonnull private Function<ProfileRequestContext,IdentifierGenerationStrategy> idGeneratorLookupStrategy;
 
-    /** JSON object mapper. */
-    @NonnullAfterInit private ObjectMapper objectMapper;
-
     /** Access control service. */
     @NonnullAfterInit private AccessControlService accessControlService;
 
@@ -180,17 +175,6 @@ public class IssueRegistrationAccessToken extends AbstractProfileAction {
         dataSealer = Constraint.isNotNull(sealer, "Data sealer cannot be null");
     }
     
-    /**
-     * Set the JSON {@link ObjectMapper}.
-     * 
-     * @param mapper object mapper
-     */
-    public void setObjectMapper(@Nonnull final ObjectMapper mapper) {
-        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
-
-        objectMapper = Constraint.isNotNull(mapper, "Object mapper cannot be null");
-    }
-
     /**
      * Set the {@link AccessControlService} to use.
      * 
@@ -347,10 +331,6 @@ public class IssueRegistrationAccessToken extends AbstractProfileAction {
             throw new ComponentInitializationException("DataSealer cannot be null");
         }
         
-        if (objectMapper == null) {
-            throw new ComponentInitializationException("ObjectMapper cannot be null");
-        }
-
         if (accessControlService == null) {
             throw new ComponentInitializationException("AccessControlService cannot be null");
         }
@@ -369,25 +349,35 @@ public class IssueRegistrationAccessToken extends AbstractProfileAction {
         }
 
         idGenerator = idGeneratorLookupStrategy.apply(profileRequestContext);
-        if (idGenerator == null) {
-            log.error("{} No identifier generation strategy", getLogPrefix());
-            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
-            return false;
-        }
 
-        issuer = issuerLookupStrategy.apply(profileRequestContext);
-        if (issuer == null) {
-            log.warn("{} No issuer could be resolved", getLogPrefix());
-            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MESSAGE);
-            return false;
-        }
+        try {
+            if (idGenerator == null) {
+                log.error("{} No identifier generation strategy", getLogPrefix());
+                sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
+                        "Internal Server Error", "System misconfiguration.");
+                return false;
+            }
+
+            issuer = issuerLookupStrategy.apply(profileRequestContext);
+            if (issuer == null) {
+                log.error("{} No issuer could be resolved", getLogPrefix());
+                sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
+                        "Internal Server Error", "System misconfiguration.");
+                return false;
+            }
 
-        policyLocation = policyLocationLookupStrategy.apply(profileRequestContext);
-        policyId = policyIdLookupStrategy.apply(profileRequestContext);
-        metadataPolicy = metadataPolicyLookupStrategy.apply(profileRequestContext);
-        if (metadataPolicy == null && policyId == null) {
-            log.warn("{} No metadata policy or policy ID could be resolved", getLogPrefix());
-            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MESSAGE);
+            policyLocation = policyLocationLookupStrategy.apply(profileRequestContext);
+            policyId = policyIdLookupStrategy.apply(profileRequestContext);
+            metadataPolicy = metadataPolicyLookupStrategy.apply(profileRequestContext);
+            if (metadataPolicy == null && policyId == null) {
+                log.warn("{} No metadata policy or policy ID could be resolved", getLogPrefix());
+                sendError(HttpServletResponse.SC_BAD_REQUEST,
+                        "Invalid Request", "No metadata policy or policy ID could be resolved.");
+                return false;
+            }
+        } catch (final IOException e) {
+            log.error("{} I/O error issuing API response", getLogPrefix(), e);
+            ActionSupport.buildEvent(profileRequestContext, EventIds.IO_ERROR);
             return false;
         }
         
@@ -444,7 +434,7 @@ public class IssueRegistrationAccessToken extends AbstractProfileAction {
         final AccessToken accessToken;
         
         try {
-            final String value = objectMapper.writeValueAsString(claimsSet);
+            final String value = getObjectMapper().writeValueAsString(claimsSet);
             log.debug("{} Built the following JSON to be sealed {}", getLogPrefix(), value);
             final String encryptedValue = dataSealer.wrap(value, claimsSet.getExpiration());
             log.debug("{} Encrypted the JSON into {}", getLogPrefix(), encryptedValue);
@@ -473,40 +463,52 @@ public class IssueRegistrationAccessToken extends AbstractProfileAction {
      * @return true iff checks pass
      */
     private boolean checkAccess(@Nonnull final ProfileRequestContext profileRequestContext) {
-        if (policyId != null) {
-            if (policyIdPolicyName == null) {
-                log.warn("{} No policy name govering policyId usage, disallowing access", getLogPrefix());
-                ActionSupport.buildEvent(profileRequestContext, EventIds.ACCESS_DENIED);
-                return false;
-            } else if (!accessControlService.getInstance(policyIdPolicyName).checkAccess(getHttpServletRequest(),
-                    "read", policyId)) {
-                ActionSupport.buildEvent(profileRequestContext, EventIds.ACCESS_DENIED);
-                return false;
+        try {
+            if (policyId != null) {
+                if (policyIdPolicyName == null) {
+                    log.warn("{} No policy name govering policyId usage, disallowing access", getLogPrefix());
+                    sendError(HttpServletResponse.SC_FORBIDDEN,
+                            "Access Denied", "No policy name govering policyId usage, disallowing access.");
+                    return false;
+                } else if (!accessControlService.getInstance(policyIdPolicyName).checkAccess(getHttpServletRequest(),
+                        "read", policyId)) {
+                    sendError(HttpServletResponse.SC_FORBIDDEN,
+                            "Access Denied", "Operation is not allowed with the current policy.");
+                    return false;
+                }
             }
-        }
 
-        if (policyLocation != null) {
-            if (policyLocationPolicyName == null) {
-                log.warn("{} No policy name govering policyLocation usage, disallowing access", getLogPrefix());
-                ActionSupport.buildEvent(profileRequestContext, EventIds.ACCESS_DENIED);
-                return false;
-            } else if (!accessControlService.getInstance(policyLocationPolicyName).checkAccess(getHttpServletRequest(),
-                    "read", policyLocation)) {
-                ActionSupport.buildEvent(profileRequestContext, EventIds.ACCESS_DENIED);
-                return false;
+            if (policyLocation != null) {
+                if (policyLocationPolicyName == null) {
+                    log.warn("{} No policy name govering policyLocation usage, disallowing access", getLogPrefix());
+                    sendError(HttpServletResponse.SC_FORBIDDEN,
+                            "Access Denied", "No policy name govering policyId usage, disallowing access.");
+                    return false;
+                } else if (!accessControlService.getInstance(policyLocationPolicyName).checkAccess(
+                        getHttpServletRequest(), "read", policyLocation)) {
+                    sendError(HttpServletResponse.SC_FORBIDDEN,
+                            "Access Denied", "Operation is not allowed with the current policy.");
+                    return false;
+                }
             }
-        }
 
-        if (clientId != null) {
-            if (clientIdPolicyName == null) {
-                log.warn("{} No policy name govering clientId usage, disallowing access", getLogPrefix());
-                ActionSupport.buildEvent(profileRequestContext, EventIds.ACCESS_DENIED);
-                return false;
-            } else if (!accessControlService.getInstance(clientIdPolicyName).checkAccess(getHttpServletRequest(),
-                    "write", clientId)) {
-                ActionSupport.buildEvent(profileRequestContext, EventIds.ACCESS_DENIED);
-                return false;
+            if (clientId != null) {
+                if (clientIdPolicyName == null) {
+                    log.warn("{} No policy name govering clientId usage, disallowing access", getLogPrefix());
+                    sendError(HttpServletResponse.SC_FORBIDDEN,
+                            "Access Denied", "No policy name govering policyId usage, disallowing access.");
+                    return false;
+                } else if (!accessControlService.getInstance(clientIdPolicyName).checkAccess(getHttpServletRequest(),
+                        "write", clientId)) {
+                    sendError(HttpServletResponse.SC_FORBIDDEN,
+                            "Access Denied", "Operation is not allowed with the current policy.");
+                    return false;
+                }
             }
+        } catch (final IOException e) {
+            log.error("{} I/O error issuing API response", getLogPrefix(), e);
+            ActionSupport.buildEvent(profileRequestContext, EventIds.IO_ERROR);
+            return false;
         }
         
         return true;
@@ -533,5 +535,5 @@ public class IssueRegistrationAccessToken extends AbstractProfileAction {
             builder.withPrincipal(subjectContext.getPrincipalName());
         }
     }
-    
+
 }
\ No newline at end of file
diff --git a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/admin/impl/IssueRegistrationAccessTokenTest.java b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/admin/impl/IssueRegistrationAccessTokenTest.java
index c20052ef..1752ed75 100644
--- a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/admin/impl/IssueRegistrationAccessTokenTest.java
+++ b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/admin/impl/IssueRegistrationAccessTokenTest.java
@@ -17,6 +17,9 @@
 
 package net.shibboleth.idp.plugin.oidc.op.admin.impl;
 
+import static org.testng.Assert.assertEquals;
+
+import java.io.UnsupportedEncodingException;
 import java.security.NoSuchAlgorithmException;
 import java.time.Duration;
 import java.time.Instant;
@@ -27,6 +30,8 @@ import javax.servlet.ServletRequest;
 
 import org.opensaml.profile.action.EventIds;
 import org.opensaml.profile.context.ProfileRequestContext;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.mock.web.MockHttpServletResponse;
 import org.springframework.webflow.execution.Event;
 import org.springframework.webflow.execution.RequestContext;
 import org.testng.Assert;
@@ -37,6 +42,7 @@ import org.testng.annotations.Test;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.JsonMappingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.github.jasminb.jsonapi.models.errors.Errors;
 import com.nimbusds.oauth2.sdk.AccessTokenResponse;
 import com.nimbusds.oauth2.sdk.token.BearerAccessToken;
 
@@ -79,6 +85,10 @@ public class IssueRegistrationAccessTokenTest {
 
     private String lifetime = "P1D";
     
+    private MockHttpServletRequest httpRequest;
+    
+    private MockHttpServletResponse httpResponse;
+    
     @BeforeClass
     public void initOnce() {
         accessControlService = new MockAccessControlService();
@@ -97,11 +107,19 @@ public class IssueRegistrationAccessTokenTest {
         action.setPolicyIdPolicyName("policyIdPolicy");
         action.setClientIdPolicyName("clientIdPolicy");
         action.setIssuerLookupStrategy(FunctionSupport.constant(issuer));
+        initRequestResponse();
         action.initialize();
         requestCtx = new RequestContextBuilder().buildRequestContext();
         prc = new WebflowRequestContextProfileRequestContextLookup().apply(requestCtx);
     }
     
+    protected void initRequestResponse() {
+        httpRequest = new MockHttpServletRequest();
+        action.setHttpServletRequest(httpRequest);
+        httpResponse = new MockHttpServletResponse();
+        action.setHttpServletResponse(httpResponse);
+    }
+    
     protected Map<String, MetadataPolicy> defaultMetadataPolicy() {
         final Map<String, MetadataPolicy> policy = new HashMap<>();
         policy.put("claim1", new MetadataPolicy.Builder().withAdd("addValue").build());
@@ -139,17 +157,20 @@ public class IssueRegistrationAccessTokenTest {
     }
 
     @Test
-    public void testNoMetadataPolicy() throws ComponentInitializationException {
+    public void testNoMetadataPolicy() throws ComponentInitializationException, JsonMappingException,
+            JsonProcessingException, UnsupportedEncodingException {
         action = new IssueRegistrationAccessToken();
         action.setSealer(dataSealer);
         action.setObjectMapper(objectMapper);
         action.setAccessControlService(accessControlService);
         action.setMetadataPolicyLookupStrategy(FunctionSupport.constant(null));
         action.setIssuerLookupStrategy(FunctionSupport.constant(issuer));
+        initRequestResponse();
         action.initialize();
         requestCtx.getFlowScope().put(IssueRegistrationAccessTokenArguments.URL_PARAM_LIFETIME, lifetime);
-        final Event event = action.execute(requestCtx);
-        ActionTestingSupport.assertEvent(event, EventIds.INVALID_MESSAGE);        
+        ActionTestingSupport.assertProceedEvent(action.execute(requestCtx));
+        assertErrorResponse(400, "Invalid Request");
+        
     }
     
     @Test
@@ -220,6 +241,21 @@ public class IssueRegistrationAccessTokenTest {
         Assert.assertTrue(instant.isBefore(target.plus(skew)));
     }
 
+    protected void assertErrorResponse(final int status, final String title)
+            throws UnsupportedEncodingException, JsonMappingException, JsonProcessingException {
+        assertEquals(httpResponse.getStatus(), status);
+        final String rawResponse = httpResponse.getContentAsString();
+        final ObjectMapper objectMapper = new ObjectMapper();
+        final Errors errors = objectMapper.readerFor(Errors.class).readValue(rawResponse);
+        Assert.assertNotNull(errors);
+        Assert.assertNotNull(errors.getErrors());
+        Assert.assertEquals(errors.getErrors().size(), 1);
+        final com.github.jasminb.jsonapi.models.errors.Error error = errors.getErrors().get(0);
+        Assert.assertNotNull(error);
+        Assert.assertEquals(error.getStatus(), "" + status);
+        Assert.assertEquals(error.getTitle(), title);
+    }
+
     /** Mock service for ACL checks. */
     private class MockAccessControlService implements AccessControlService {
 
diff --git a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/IssueRegistrationAccessTokenFlowTest.java b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/IssueRegistrationAccessTokenFlowTest.java
index c6acc0bd..b0c09ac3 100644
--- a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/IssueRegistrationAccessTokenFlowTest.java
+++ b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/IssueRegistrationAccessTokenFlowTest.java
@@ -30,6 +30,10 @@ import org.springframework.webflow.executor.FlowExecutionResult;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonMappingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.github.jasminb.jsonapi.models.errors.Errors;
 import com.nimbusds.oauth2.sdk.AccessTokenResponse;
 
 import net.shibboleth.idp.plugin.oidc.op.cli.IssueRegistrationAccessTokenArguments;
@@ -60,9 +64,9 @@ public class IssueRegistrationAccessTokenFlowTest extends AbstractOidcFlowTest {
         final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
 
         final FlowExecutionOutcome outcome = result.getOutcome();
-        //assertEquals(response.getStatus(), 300);
-        //assertEquals(outcome.getId(), "ErrorView");
-        //TODO: implement the error handling to the flow
+        assertEquals(outcome.getId(), "CommitResponse");
+
+        assertErrorResponse(400, "Invalid Request");
     }
 
     /**
@@ -104,4 +108,19 @@ public class IssueRegistrationAccessTokenFlowTest extends AbstractOidcFlowTest {
             request.addParameter(IssueRegistrationAccessTokenArguments.URL_PARAM_POLICY_ID, relyingPartyId);
         }
     }
+    
+    private void assertErrorResponse(final int status, final String title)
+            throws UnsupportedEncodingException, JsonMappingException, JsonProcessingException {
+        assertEquals(response.getStatus(), status);
+        final String rawResponse = response.getContentAsString();
+        final ObjectMapper objectMapper = new ObjectMapper();
+        final Errors errors = objectMapper.readerFor(Errors.class).readValue(rawResponse);
+        Assert.assertNotNull(errors);
+        Assert.assertNotNull(errors.getErrors());
+        Assert.assertEquals(errors.getErrors().size(), 1);
+        final com.github.jasminb.jsonapi.models.errors.Error error = errors.getErrors().get(0);
+        Assert.assertNotNull(error);
+        Assert.assertEquals(error.getStatus(), "" + status);
+        Assert.assertEquals(error.getTitle(), title);
+    }
 }

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list