ilgrosso commented on a change in pull request #181:
URL: https://github.com/apache/syncope/pull/181#discussion_r422834916
##########
File path: wa/starter/pom.xml
##########
@@ -237,12 +241,14 @@ under the License.
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-security</artifactId>
</dependency>
-
+ <dependency>
Review comment:
Not a big issue, but it would be nice to separate groups of dependencies
by a blank line: e.g. can you add one above this?
##########
File path: wa/starter/pom.xml
##########
@@ -237,12 +241,14 @@ under the License.
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-security</artifactId>
</dependency>
-
+ <dependency>
+ <groupId>org.bouncycastle</groupId>
+ <artifactId>bcprov-jdk15on</artifactId>
+ </dependency>
<dependency>
<groupId>org.bouncycastle</groupId>
<artifactId>bcpkix-jdk15on</artifactId>
</dependency>
-
Review comment:
Not a big issue, but it would be nice to separate groups of dependencies
by a blank line: e.g. can you restore the blank line here?
##########
File path:
wa/starter/src/test/java/org/apache/syncope/wa/pac4j/saml/BaseSyncopeWASAML2Client.java
##########
@@ -0,0 +1,117 @@
+/*
Review comment:
Can you move this package under `org.apache.syncope.wa.starter` which
should be the root package of this module?
##########
File path:
common/am/rest-api/src/main/java/org/apache/syncope/common/rest/api/service/SAML2SPMetadataConfService.java
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF 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 org.apache.syncope.common.rest.api.service;
+
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.enums.ParameterIn;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.responses.ApiResponse;
+import io.swagger.v3.oas.annotations.responses.ApiResponses;
+import io.swagger.v3.oas.annotations.security.SecurityRequirement;
+import io.swagger.v3.oas.annotations.security.SecurityRequirements;
+import io.swagger.v3.oas.annotations.tags.Tag;
+import org.apache.syncope.common.lib.to.SAML2SPMetadataTO;
+import org.apache.syncope.common.rest.api.RESTHeaders;
+
+import javax.validation.constraints.NotNull;
+import javax.ws.rs.Consumes;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.Produces;
+import javax.ws.rs.core.MediaType;
+
+/**
+ * REST operations for SAML 2.0 SP metadata.
+ */
+@Tag(name = "SAML 2.0 SP Metadata")
+@SecurityRequirements({
+ @SecurityRequirement(name = "BasicAuthentication"),
+ @SecurityRequirement(name = "Bearer")})
+@Path("saml2sp/conf/metadata")
Review comment:
What it the difference to this service bound under
`saml2sp/conf/metadata` and the other one bound under "saml2sp/metadata"?
##########
File path:
common/am/rest-api/src/main/java/org/apache/syncope/common/rest/api/service/SAML2SPKeystoreConfService.java
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF 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 org.apache.syncope.common.rest.api.service;
+
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.enums.ParameterIn;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.responses.ApiResponse;
+import io.swagger.v3.oas.annotations.responses.ApiResponses;
+import io.swagger.v3.oas.annotations.security.SecurityRequirement;
+import io.swagger.v3.oas.annotations.security.SecurityRequirements;
+import io.swagger.v3.oas.annotations.tags.Tag;
+import org.apache.syncope.common.lib.to.SAML2SPKeystoreTO;
+import org.apache.syncope.common.rest.api.RESTHeaders;
+
+import javax.validation.constraints.NotNull;
+import javax.ws.rs.Consumes;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.Produces;
+import javax.ws.rs.core.MediaType;
+
+/**
+ * REST operations for SAML 2.0 SP Keystore.
+ */
+@Tag(name = "SAML 2.0 SP Keystore")
+@SecurityRequirements({
+ @SecurityRequirement(name = "BasicAuthentication"),
+ @SecurityRequirement(name = "Bearer") })
+@Path("saml2sp/conf/keystore")
+public interface SAML2SPKeystoreConfService extends JAXRSService {
Review comment:
What it the difference to this service bound under
`saml2sp/conf/keystore` and the other one bound under "saml2sp/keystore"?
##########
File path:
common/am/lib/src/main/java/org/apache/syncope/common/lib/types/AMEntitlement.java
##########
@@ -60,6 +60,18 @@
public static final String SAML2_IDP_METADATA_READ =
"SAML2_IDP_METADATA_READ";
+ public static final String SAML2_SP_METADATA_CREATE =
"SAML2_SP_METADATA_CREATE";
Review comment:
So SAML metadata and keystore cannot be deleted?
##########
File path:
common/am/rest-api/src/main/java/org/apache/syncope/common/rest/api/service/SAML2SPMetadataService.java
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF 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 org.apache.syncope.common.rest.api.service;
+
+import io.swagger.v3.oas.annotations.headers.Header;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.responses.ApiResponse;
+import io.swagger.v3.oas.annotations.responses.ApiResponses;
+import io.swagger.v3.oas.annotations.security.SecurityRequirement;
+import io.swagger.v3.oas.annotations.security.SecurityRequirements;
+import io.swagger.v3.oas.annotations.tags.Tag;
+import org.apache.syncope.common.lib.to.SAML2SPMetadataTO;
+import org.apache.syncope.common.rest.api.RESTHeaders;
+
+import javax.validation.constraints.NotNull;
+import javax.ws.rs.Consumes;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.HttpHeaders;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+
+/**
+ * REST operations for SAML 2.0 SP metadata.
+ */
+@Tag(name = "SAML 2.0 SP Metadata")
+@SecurityRequirements({
+ @SecurityRequirement(name = "BasicAuthentication"),
+ @SecurityRequirement(name = "Bearer") })
+@Path("saml2sp/metadata")
+public interface SAML2SPMetadataService extends JAXRSService {
+
+ /**
+ * Returns a document outlining metadata for Syncope as SAML 2.0 SP.
+ *
+ * @param name indicates the SAML 2.0 SP metadata document owner.
+ * @return SAML 2.0 SP metadata
+ */
+ @GET
+ @Produces({ MediaType.APPLICATION_JSON, RESTHeaders.APPLICATION_YAML,
MediaType.APPLICATION_XML })
+ SAML2SPMetadataTO get(@QueryParam("name") String name);
+
+ /**
+ * Returns the SAML 2.0 SP metadata matching the given key.
+ *
+ * @param key key of requested SAML 2.0 SP metadata
+ * @return SAML 2.0 SP metadata with matching id
+ */
+ @GET
+ @Path("{key}")
+ @Produces({ MediaType.APPLICATION_JSON, RESTHeaders.APPLICATION_YAML,
MediaType.APPLICATION_XML })
+ SAML2SPMetadataTO read(@NotNull @PathParam("key") String key);
Review comment:
What is the difference between `get()` and `read()`? I think it should
be made more clear by reviewing the REST mapping.
##########
File path:
common/am/rest-api/src/main/java/org/apache/syncope/common/rest/api/service/SAML2SPKeystoreService.java
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF 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 org.apache.syncope.common.rest.api.service;
+
+import io.swagger.v3.oas.annotations.headers.Header;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.responses.ApiResponse;
+import io.swagger.v3.oas.annotations.responses.ApiResponses;
+import io.swagger.v3.oas.annotations.security.SecurityRequirement;
+import io.swagger.v3.oas.annotations.security.SecurityRequirements;
+import io.swagger.v3.oas.annotations.tags.Tag;
+import org.apache.syncope.common.lib.to.SAML2SPKeystoreTO;
+import org.apache.syncope.common.rest.api.RESTHeaders;
+
+import javax.validation.constraints.NotNull;
+import javax.ws.rs.Consumes;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.HttpHeaders;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+
+/**
+ * REST operations for SAML 2.0 IdP keystore.
+ */
+@Tag(name = "SAML 2.0 SP Metadata Keystore")
+@SecurityRequirements({
+ @SecurityRequirement(name = "BasicAuthentication"),
+ @SecurityRequirement(name = "Bearer") })
+@Path("saml2sp/keystore")
+public interface SAML2SPKeystoreService extends JAXRSService {
+
+ /**
+ * Returns a document outlining keystore for Syncope as SAML 2.0 SP.
+ *
+ * @param name indicates the SAML 2.0 SP keystore document owner.
+ * @return SAML 2.0 SP keystore
+ */
+ @GET
+ @Produces({ MediaType.APPLICATION_JSON, RESTHeaders.APPLICATION_YAML,
MediaType.APPLICATION_XML })
+ SAML2SPKeystoreTO get(@QueryParam("name") String name);
+
+ /**
+ * Returns the SAML 2.0 SP keystore matching the given key.
+ *
+ * @param key key of requested SAML 2.0 SP keystore
+ * @return SAML 2.0 SP keystore with matching id
+ */
+ @GET
+ @Path("{key}")
+ @Produces({ MediaType.APPLICATION_JSON, RESTHeaders.APPLICATION_YAML,
MediaType.APPLICATION_XML })
+ SAML2SPKeystoreTO read(@NotNull @PathParam("key") String key);
Review comment:
What is the difference between `get()` and `read()`? I think it should
be made more clear by reviewing the REST mapping.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
