Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enhancement/wip java11 #19

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Enhancement/wip java11 #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1d326d6
remove unused code
Jul 25, 2021
a5f7ec7
adding test
FaberNa Jul 25, 2021
e34b4df
change travis file
FaberNa Jul 25, 2021
22b4286
adding test for travis
FaberNa Jul 25, 2021
e7924c6
change travis for test
FaberNa Jul 25, 2021
993dab0
Revert "change travis for test"
FaberNa Jul 25, 2021
8fdf295
Revert "adding test for travis"
FaberNa Jul 25, 2021
350d297
adding fix for test
FaberNa Jul 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# add dist trusty for jdk8
# add dist trusty for jdk11
dist: trusty

language: java

jdk:
- oraclejdk8
- oraclejdk11
21 changes: 19 additions & 2 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,27 @@
<artifactId>maven-compiler-plugin</artifactId>
<version>3.7.0</version>
<configuration>
<source>1.8</source>
<target>1.8</target>
<source>11</source>
<target>11</target>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>2.19.1</version>
<dependencies>
<dependency>
<groupId>org.junit.platform</groupId>
<artifactId>junit-platform-surefire-provider</artifactId>
<version>1.1.0</version>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-engine</artifactId>
<version>5.1.0</version>
</dependency>
</dependencies>
</plugin>
</plugins>
</build>

Expand Down
77 changes: 77 additions & 0 deletions ...gration/src/test/java/it/italia/developers/spid/integration/config/IdpKeyManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package it.italia.developers.spid.integration.config;

import lombok.SneakyThrows;
import org.assertj.core.api.SoftAssertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.opensaml.xml.security.credential.Credential;

import java.security.cert.Certificate;

import static org.assertj.core.api.Assertions.assertThat;

class IdpKeyManagerTest {

static IdpKeyManager idpKeyManager;

@SneakyThrows
@BeforeAll
static void startup(){
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect indentation

Suggested change
static void startup(){
static void startup(){

String entity ="test.idp.entity";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a space

Suggested change
String entity ="test.idp.entity";
String entity = "test.idp.entity";

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This string is repeated a few times throughout the tests, consider extracting it to a constant.

String certificateStr = "-----BEGIN CERTIFICATE-----\n" +
"MIICmDCCAgGgAwIBAgIBADANBgkqhkiG9w0BAQ0FADBpMQswCQYDVQQGEwJpdDEP\n" +
"MA0GA1UECAwGVG9yaW5vMQ0wCwYDVQQKDARTcGlkMQ0wCwYDVQQDDARDYXRhMQ0w\n" +
"CwYDVQQLDARzcGlkMRwwGgYJKoZIhvcNAQkBFg1mYWJlckBtYWlsLml0MB4XDTIx\n" +
"MDcyNTE2MzA0M1oXDTIyMDcyNTE2MzA0M1owaTELMAkGA1UEBhMCaXQxDzANBgNV\n" +
"BAgMBlRvcmlubzENMAsGA1UECgwEU3BpZDENMAsGA1UEAwwEQ2F0YTENMAsGA1UE\n" +
"CwwEc3BpZDEcMBoGCSqGSIb3DQEJARYNZmFiZXJAbWFpbC5pdDCBnzANBgkqhkiG\n" +
"9w0BAQEFAAOBjQAwgYkCgYEAxVPBzk/AHxSdsXhZ9kRbc9r2pUoRTDHEIts7BCjW\n" +
"gJSEHjRKOJG58Sx+LMv9feoStNNsR7XN/mNGFliq6mRucui1cdc+M6lP//ZjR5Al\n" +
"m4LRQhQiDXl2Hr5WD4+vgM3UH2zQTftawXBQO/VI8FHnKOMT55b4jtgSI12+U5OW\n" +
"EQ0CAwEAAaNQME4wHQYDVR0OBBYEFIF6KK7ydM5V1rXlj/eh3RM7QuooMB8GA1Ud\n" +
"IwQYMBaAFIF6KK7ydM5V1rXlj/eh3RM7QuooMAwGA1UdEwQFMAMBAf8wDQYJKoZI\n" +
"hvcNAQENBQADgYEArHyy3+ju6aHDkfFmKIOhWEMCutUQiC7/8HXa+Y4EkHkf5nsO\n" +
"/aU5lv9UoFPfSIGTPJKBC1mMYU9ybHXI8dRgBvbrKhp59jimBd9BknFB7qeJZW07\n" +
"4TvnsfrDA6QC7Hon4Q9PB6ARbirg31T6xHq9XJpUMEGvV5GEqA2hKhJkkJk=\n" +
"-----END CERTIFICATE-----";
idpKeyManager = new IdpKeyManager(entity,certificateStr);
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra line

Suggested change


@Test
void shouldReturnCredentialWhenKeyisNotNullAndKeyEqualEntity() {
Credential credential = idpKeyManager.getCredential("test.idp.entity");
SoftAssertions.assertSoftly(softly -> {
softly.assertThat(credential).isNotNull();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered whether this assertion could be redundant? Since if credential will ever be null, the next assertion would fail the test anyway.

softly.assertThat(credential.getEntityId()).isEqualTo("test.idp.entity");
});
}
@Test
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing an empty line

Suggested change
@Test
@Test

void shouldReturnNullCredentialWhenKeyisNull() {
Credential credential = idpKeyManager.getCredential(null);
assertThat(credential).isNull();
}
@Test
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing an empty line

Suggested change
@Test
@Test

void shouldReturnNullCredentialWhenKeyisNotEqualToEntity() {
Credential credential = idpKeyManager.getCredential("test");
assertThat(credential).isNull();
}

@Test
void shouldReturnCertificateWhenKeyisNotNullAndKeyEqualEntity() {
Certificate certificate = idpKeyManager.getCertificate("test.idp.entity");
assertThat(certificate).isNotNull();
}

@Test
void shouldReturnNullCertificateWhenKeyisNull() {
Certificate certificate = idpKeyManager.getCertificate(null);
assertThat(certificate).isNull();
}
@Test
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing an empty line

Suggested change
@Test
@Test

void shouldReturnNullCertificateWhenKeyisNotEqualToEntity() {
Certificate certificate = idpKeyManager.getCertificate("test");
assertThat(certificate).isNull();
}

}
6 changes: 3 additions & 3 deletions ...t/java/it/italia/developers/spid/integration/service/test/SPIDIntegrationServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public void shouldGetSpecificXMLWhenCallAuthRequest() throws Exception {


AuthRequest authRequest = spidIntegrationService.buildAuthenticationRequest("idp.spid.gov.it", 0);
String result = spidIntegrationUtil.decode(authRequest.getXmlAuthRequest());
String result = authRequest.getXmlAuthRequest();

Element resultElement = spidIntegrationUtil.xmlStringToElement(result);

Expand All @@ -70,7 +70,7 @@ public void shouldGetIdWhenCallAuthRequest() throws Exception {
String IDPEntityId = "idp.spid.gov.it";
int assertionConsumerServiceIndex = 0;
AuthRequest authRequest = spidIntegrationService.buildAuthenticationRequest(IDPEntityId, assertionConsumerServiceIndex);
String result = spidIntegrationUtil.decode(authRequest.getXmlAuthRequest());
String result = authRequest.getXmlAuthRequest();

Element resultElement = spidIntegrationUtil.xmlStringToElement(result);
SoftAssertions.assertSoftly(softly -> {
Expand All @@ -80,7 +80,7 @@ public void shouldGetIdWhenCallAuthRequest() throws Exception {
softly.assertThat(resultElement.getAttributes().getNamedItem("Destination").getTextContent()).contains(IDPEntityId);
softly.assertThat(resultElement.getAttributes().getNamedItem("AssertionConsumerServiceIndex").getTextContent()).isEqualTo(String.valueOf(assertionConsumerServiceIndex));
softly.assertThat(resultElement.getAttributes().getNamedItem("ProtocolBinding").getTextContent()).isEqualTo("urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST");
softly.assertThat(resultElement.getAttributes().getNamedItem("IsPassive")).isNull();
softly.assertThat(resultElement.getAttributes().getNamedItem("IsPassive")).isNotNull();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FaberNa mi pareva di aver capito (anche da questa guida qui) che l'attributo IsPassive non dovesse essere presente.

@peppelinux tu sei sicuramente più informato.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marque88 è corretto

consiglio di controllare metadata e authnrequest con spid-sp-test per stare tranquilli su eventuali anomalie da risolvere

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • nell’elemento non deve essere presente l’attributo IsPassive (ad indicare false come valore di default) ... in verità andrebbe aggiunto che se presente deve essere false

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

un giro con spid-sp-test ci darebbe contezza sulle criticità, esempio:

https://github.com/italia/spid-sp-test/blob/8b8dc56884cfcdb2a218e975a4738318ef87f408/src/spid_sp_test/authn_request.py#L523

Questi sono i controlli che AgID fa in fase di collaudo

});
}

Expand Down