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.

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

Problemi nella generazione del metadata #11

Closed
matteo2411 opened this issue Mar 8, 2021 · 13 comments
Closed

Problemi nella generazione del metadata #11

matteo2411 opened this issue Mar 8, 2021 · 13 comments
Labels
question Further information is requested waiting for feedback Waiting from feedback from the reporter

Comments

@matteo2411
Copy link

Il metadata generato non contiene lo User Name attribute, la parte relativa a ContactPerson e in AttributeConsumingService manca il tag ServiceDescription. Inoltre nei KeyInfo è rimasto il KeyName il quale, a meno di modifiche su idptest e testenv2 non dovrebbe essere accettato se non sbaglio

@lscorcia
Copy link
Collaborator

lscorcia commented Mar 8, 2021

Buongiorno Matteo,
lo user name non è un campo restituito da SPID; se ti riferisci al fatto che manca il fiscalNumber, nel wiki suggerisco di mappare lo stesso attributo due volte, la prima come UserName Template Importer e la seconda come Attribute Importer, in modo da mantenere la differenza semantica tra questi due attributi.

ContactPerson dovrebbe essere alimentato; puoi verificare se hai inserito i dati corrispondenti nel primo IdP in ordine alfabetico per alias? I dati "condivisi" tra più IdP vengono ricavati solo dal primo IdP in ordine alfabetico.

https://login.domain.com:8443/auth/realms/spid/spid-sp-metadata
immagine

ServiceDescription non è presente, confermo. Serve a poco o niente, ma se hai un requisito per cui ti può essere utile lo aggiungo...

Per la questione del KeyName, si tratta di un bug di spid-testenv2 già tracciato qui: italia/spid-testenv2#325 , quindi non modificherò questa libreria. In ogni caso, suggerisco l'uso di spid-saml-check anziché di spid-testenv2.

@matteo2411
Copy link
Author

matteo2411 commented Mar 8, 2021

Si mi ero dimenticato di configurare tutto sul primo Idp, ora è ok.
Per quanto riguarda il ServiceDescription non credo mi serva, ricordavo fosse un controllo dello spid-saml-check ma probabilmente sbagliavo. Per la questione del KeyName ovviamente quel bug è bloccante e ci permette il test solo tramite spid-saml-check.

@lscorcia
Copy link
Collaborator

lscorcia commented Mar 8, 2021

Per fortuna AGID usa solo spid-saml-check per effettuare i controlli ;)
Ho comunque inviato la correzione per spid-testenv2 al progetto... prima o poi la includeranno. Intanto, se vuoi puoi compilare il docker importando la PR italia/spid-testenv2#327 e funzionerà.

@matteo2411
Copy link
Author

Però sono due errori diversi, uno e sul "Both X509Data and KeyValue found" e uno è sulla presenza o meno del KeyName nella parte relativa ai certificati

@erapsag
Copy link

erapsag commented Mar 9, 2021

a proposito del metadata generato in automatico, ho riscontrato che genera un solo <md: AttributeConsumingService index="x">.
Questo significa che nelle configurazioni dei vari IdP devono avere tutti il parametro "Attribute Consuming Service Index" settato a quello stesso valore "x"? e non che deve coincidere con gli index dei vari <md: AssertionConsumerService> che sono invece differenti per tutti gli IdP

@lscorcia
Copy link
Collaborator

lscorcia commented Mar 9, 2021

@matteo2411 , anche il problema sul KeyName è un bug in spid-testenv2, questo però è già stato corretto: italia/spid-testenv2#326 . Non è ancora stata fatta una release ufficiale con questa fix quindi se vuoi usarla devi compilare il docker partendo dai sorgenti.

@erapsag : esatto, tutti gli IdP devono condividere lo stesso valore di AttributeConsumingServiceIndex (e in effetti non avrebbe senso che alcuni IdP restituissero attributi diversi dagli altri). Non c'è nessuna relazione tra AttributeConsumingServiceIndex e l'index degli AssertionConsumerService. Se vuoi approfondiamo meglio in un'altra issue, cerchiamo di tenere il thread pulito :)

@lscorcia lscorcia added the question Further information is requested label Mar 10, 2021
@matteo2411
Copy link
Author

il bug del KeyName corretto è sul signature e non nel KeyInfo del KeyDescriptor o sbaglio? Ho questo errore:

image

@lscorcia
Copy link
Collaborator

Ciao @matteo2411 , scusa il forte ritardo nella risposta, ho dovuto accantonare il progetto per qualche settimana. L'errore è stato corretto nel generico elemento KeyDescriptor, quindi la issue dovrebbe risolvere il tuo problema (ma non hanno ancora effettuato una nuova release, quindi sempre dai sorgenti...). In ogni caso, spid-testenv2 è ormai davvero obsoleto e ti consiglio caldamente di passare a spid-saml-check per dei test più realistici.

Per maggiori info sui vari validatori e progetti puoi rivolgerti al canale slack Developers Italia > SPID https://developersitalia.slack.com/archives/C73R3UQE8 .

@freddy34
Copy link

spid-saml-check ha ritornato un errore semantico relativo alla struttura del tag ContactPerson type="billing" in quanto non allineato alle ultime indicazioni delle regole tecniche:

  • Extensions (1 occorrenza obbligatorio)
    • CessionarioCommittente (1 occorrenza) — con figli:
      • DatiAnagrafici (1 occorrenza) — con figli:
        • IdFiscaleIVA
        • ...
      • Sede (1 occorrenza) — con figli:
        • Indirizzo
        • ...
    • TerzoIntermediarioSoggettoEmittente (0 o 1 occorrenze) — valorizzato, se necessario e solo relativamente al committente.
  • Company (0 o 1 occorrenze)
  • EmailAddress (1 occorrenza, obbligatorio)

attualmente il nodo viene generato con i seguenti tag (old-style):

  • md:Company
  • md:EmailAddress
  • md:TelephoneNumber

corretto?

@lscorcia
Copy link
Collaborator

Ciao @freddy34 , confermo quanto dici: non ho implementato la miriade di estensioni richieste per i soggetti privati, più che altro perché lavoro per una PA e non ne ho avuto necessità; cercherò di dare una occhiata alla specifica e vedrò cosa si può fare.
In linea di principio mi piacerebbe poter risolvere il problema alla radice aggiungendo un campo "XML a piacere da aggiungere alle Extensions", così da gestire eventuali variazioni future senza impazzire...

@mmariniopensoftware
Copy link

Ciao @freddy34 @lscorcia
ho letto questo messaggio un paio di settimane fa e avevo anch'io lo stesso problema dato che la mia azienda non è una PA (verificando con spid-saml-check falliva la validazione del metadato e di conseguenza bloccava anche la login) quindi ho aggiunto i campi del ContactType 'billing' che mancavano nella form di modifica dell'IDP(appaiono solo abilitando SP come 'Private')

Ho preso come riferimento per il metadata la nota linkata dal sito dell'agid
(https://www.agid.gov.it/sites/default/files/repository_files/spid-avviso-n19v4-regole_tecniche_aggregatori_0.pdf

Ho pushato le modifiche sul mio branch qui
https://github.com/mmariniopensoftware/spid-keycloak-provider/tree/add-some-info-private-sp

Non sapevo se crearti una PR dato che le modifiche che ho fatto non so quanto sono consistenti con il master. Non conosco bene le specifiche e tutto ciò che riguarda il mondo agid/spid per cui mi sono limitato ad aggiungere i campi che mi servivano nella form, con un unica stringa xml come proponevi te non sono riuscito dato che per i privati vanno aggiunti dei tag aggiuntivi anche sul ContactType Other (se non mi ricordo male.. dovrei rifare dei test con il validatore dato che ho fatto la modifica un paio di settimane fa)

In ogni caso grazie @lscorcia per il plugin, se nn avremo inconvenienti contiamo di andare in produzione nei prossimi mesi

@lscorcia
Copy link
Collaborator

Ciao, questa richiesta è stata implementata tramite la PR #30 inclusa nella release 1.0.12. Vi prego di farmi sapere se manca qualcos'altro e grazie @mmariniopensoftware per l'implementazione iniziale!

@lscorcia lscorcia added the waiting for feedback Waiting from feedback from the reporter label Feb 24, 2022
@nicolabeghin
Copy link
Collaborator

Chiudo in quanto mergiato in #30

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested waiting for feedback Waiting from feedback from the reporter
Projects
None yet
Development

No branches or pull requests

6 participants