search

italia / spid-express

Express middleware implementing SPID & Entra con CIE (Carta d'Identità Elettronica)
MIT License
41 stars 16 forks source link

Introduced HTTPS, private sp ContactPerson nodes on metadata, fix for /error without 200 http code #43

Closed matfur92 closed 3 years ago

matfur92 commented 3 years ago

In order to reach the certification for a private service provider, here a set of feature that I've implemented:

There are still some problems as before this pull request. Here below a list of errors found with spid-sp-test v.0.6.3 ( thanks to @peppelinux for the support)

29 failure RelayState is missing - TR pag. 14 or pag. 15 Missing RelayState
31 failure AuthnRequest Signature validation failed Verification Failure
42 failure The ForceAuthn attribute MUST be present because of minimum/SpidL1 {'ID': '_727023c5dd22a8f92fa0', 'Version': '2.0', 'IssueInstant': '2021-06-02T14:27:56.935Z', 'ProtocolBinding': 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST', 'Destination': 'https://domainspidtestenv2/sso', 'AssertionConsumerServiceURL': 'https://domainspidexpress/acs', 'AttributeConsumingServiceIndex': '0'}
43 failure The ForceAuthn attribute MUST be True because of minimum/SpidL1 {'ID': '_727023c5dd22a8f92fa0', 'Version': '2.0', 'IssueInstant': '2021-06-02T14:27:56.935Z', 'ProtocolBinding': 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST', 'Destination': 'https://domainspidtestenv2/sso', 'AssertionConsumerServiceURL': 'https://domainspidexpress/acs', 'AttributeConsumingServiceIndex': '0'}
1 failure Test [1] Response corretta. Risultato atteso: Ok: [http status_code: 500]
27 failure Test [31] L'attributo Format di Issuer deve essere omesso o assumere valore urn:oasis:names:tc:SAML:2.0:nameid-format:entity. In questo test il valore è omesso. Risultato atteso: Ok: [http status_code: 500]
86 failure Test [94] Elemento AuthContextClassRef impostato su https://www.spid.gov.it/SpidL1. Risultato atteso: fare attenzione al livello richiesto sulla request.: [http status_code: 500]
87 failure Test [95] Elemento AuthContextClassRef impostato su https://www.spid.gov.it/SpidL2. Risultato atteso: fare attenzione al livello richiesto sulla request.: [http status_code: 500]
88 failure Test [96] Elemento AuthContextClassRef impostato su https://www.spid.gov.it/SpidL3. Risultato atteso: fare attenzione al livello richiesto sulla request.: [http status_code: 500]
99 failure Test [109] Response corretta. Risultato atteso: Ok: [http status_code: 500]
100 failure Test [110] Attributo IssueInstant specificato con millisecondi. Risultato atteso: Ok: [http status_code: 500]
peppelinux commented 3 years ago

Authn test related to forceauthn probably are wrong, forceauthn Is needed only with GTE L2

Relaystate should be trivial to be implemented ...

Regarding response tests ... The First test, Number 1, MUST return 200, It's instead 500 😶

Thank you @matfur92, this Is a good starting point

peppelinux commented 3 years ago

@matfur92 CI fails, that's something that must be investigated before considering these commit ready to be merged

@bfabio Is there any clue on this?

bfabio commented 3 years ago

@peppelinux yarn.lock needs to be updated in the PR (with yarn install)

@matfur92 thanks for the PR! In general we are trying to be as close to upstream as possible by merging their changes and upstreaming ours. I haven't managed to compare the repos yet, but AFAIU, correct me if I'm wrong, https://github.com/pagopa/io-spid-commons/pull/81 is implementing part of the changes in this PR and I think we should pull that first.