-
Notifications
You must be signed in to change notification settings - Fork 22
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.
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
Enhancement/wip java11 #19
base: master
Are you sure you want to change the base?
Changes from all commits
1d326d6
a5f7ec7
e34b4df
22b4286
e7924c6
993dab0
8fdf295
350d297
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 |
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(){ | ||||||||
String entity ="test.idp.entity"; | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing a space
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||||||||
} | ||||||||
|
||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(); | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you considered whether this assertion could be redundant? Since if |
||||||||
softly.assertThat(credential.getEntityId()).isEqualTo("test.idp.entity"); | ||||||||
}); | ||||||||
} | ||||||||
@Test | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing an empty line
Suggested change
|
||||||||
void shouldReturnNullCredentialWhenKeyisNull() { | ||||||||
Credential credential = idpKeyManager.getCredential(null); | ||||||||
assertThat(credential).isNull(); | ||||||||
} | ||||||||
@Test | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing an empty line
Suggested change
|
||||||||
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 | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing an empty line
Suggested change
|
||||||||
void shouldReturnNullCertificateWhenKeyisNotEqualToEntity() { | ||||||||
Certificate certificate = idpKeyManager.getCertificate("test"); | ||||||||
assertThat(certificate).isNull(); | ||||||||
} | ||||||||
|
||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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); | ||
|
||
|
@@ -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 -> { | ||
|
@@ -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(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. un giro con spid-sp-test ci darebbe contezza sulle criticità, esempio: Questi sono i controlli che AgID fa in fase di collaudo |
||
}); | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect indentation