italia / spid-sp-test

SAML2 SPID/CIE Service Provider validation tool
European Union Public License 1.2
38 stars 17 forks source link

Invalid check on private service provider #38

Closed matfur92 closed 3 years ago

matfur92 commented 3 years ago

Referring to https://www.agid.gov.it/sites/default/files/repository_files/spid-avviso-n29v3-specifiche_sp_pubblici_e_privati.pdf there are two checks for --profile spid-sp-private that are not correct.

1. on page 6 of the linked pdf, there is the same structure reported on https://docs.italia.it/italia/spid/spid-regole-tecniche/it/stabile/metadata.html#esempio-contatti-metadata-sp-per-fatturazione that has EmailAddress and TelephoneNumber out of Extensions node:

   <md:ContactPerson contactType="other">
        <md:Extensions>
            <spid:VATNumber>IT12345678901</spid:VATNumber>
            <spid:FiscalCode>XYZABCAAMGGJ000W</spid:FiscalCode>
            <spid:Private/>
        </md:Extensions>
        <md:EmailAddress>spid@organizzazione.com</md:EmailAddress>
        <md:TelephoneNumber>+390123456789</md:TelephoneNumber>
    </md:ContactPerson>
    <md:ContactPerson contactType="billing">
        <md:Extensions 
               xmlns:fpa="https://spid.gov.it/invoicing-extensions">
            <fpa:CessionarioCommittente>
                <fpa:DatiAnagrafici>
                    <fpa:IdFiscaleIVA>
                        <fpa:IdPaese>IT</fpa:IdPaese>
                        <fpa:IdCodice>02468135791</fpa:IdCodice>
                    </fpa:IdFiscaleIVA>
                    <fpa:Anagrafica>
                       <fpa:Denominazione>
                            Destinatario_Fatturazione
                       </fpa:Denominazione> 
                    </fpa:Anagrafica>
                </fpa:DatiAnagrafici>
                <fpa:Sede>
                     <fpa:Indirizzo>via [...]</fpa:Indirizzo>
                     <fpa:NumeroCivico>99</fpa:NumeroCivico>
                     <fpa:CAP>12345</ fpa:CAP>
                     <fpa:Comune>nome_citta</fpa:Comune>
                     <fpa:Provincia>XY</fpa:Provincia>
                    <fpa:Nazione>IT</fpa:Nazione>
                </fpa:Sede>
            </fpa:CessionarioCommittente>
        </md:Extensions>
        <md:Company>Destinatario_Fatturazione</md:Company>
        <md:EmailAddress>email@fatturazione.it</md:EmailAddress>
        <md:TelephoneNumber>telefono_fatture</md:TelephoneNumber>
    </md:ContactPerson>

Inside spid-sp-test/src/spid_sp_test/metadata_private.py there is the check that validate the EmailAddress as child of Extensions as reported into my test run:

INFO:spid_sp_test.metadata:SpidSpMetadataCheckExtra.test_extentions_public
ERROR:spid_sp_test.metadata:The //ContactPerson/Extensions/CessionarioCommittente/EmailAddress element MUST be present

This is not correct like reported into the documentation.

2. for private service provider there is:

INFO:spid_sp_test.metadata:SpidSpMetadataCheckExtra.test_Contacts_PubPriv
ERROR:spid_sp_test.metadata:Only one Extensions element inside ContactPerson element MUST be present

but probably, having two nodes ContactPerson (other and billing), this test count two times the Extensions and this is not correct.

peppelinux commented 3 years ago

@matfur92 domani aggiornerò gli esempi di metadata su docs Italia. Controlla gli esempi degli avvisi spid e se trovi difetto anche lì ti prego di riportarli

Farò alla stessa maniera anch'io

peppelinux commented 3 years ago

Regarding 1. Yes, phone and email are standards fields standing on Saml2 contactperson. I have to check Cessionario committente, i think that this MAY be there and MUST be checked only if defined

@damikael put a spell on this, I'll procede with a patch

peppelinux commented 3 years ago

Regarding 2. Yes, I think that you're right

@damikael please give a confirmation on that, I'll check specs asap

peppelinux commented 3 years ago

@matfur92 ok after some guru meditation I can say that:

  1. //ContactPerson/Extensions/CessionarioCommittente/EmailAddress SHOULD not exists -> removed!
  2. My fault, no more than a single Extension inside each ContactPerson should be handled -> fixed!

if @damikael doesn't have anything to say I consider this close (reopen if necessary).

this release is dedicated to you and with gratitude @matfur92 https://github.com/italia/spid-sp-test/releases/tag/v0.6.1

damikael commented 3 years ago

No more to say. Issues pointed by @matfur92 are correct. Good work guys!