search

italia / spid-keycloak-provider

Italian SPID authentication provider for Keycloak (https://www.keycloak.org/)
Apache License 2.0
63 stars 22 forks source link

Problemi nella generazione del metadata #11

Closed matteo2411 closed 8 months ago

matteo2411 commented 3 years ago

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 commented 3 years ago

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: https://github.com/italia/spid-testenv2/issues/325 , quindi non modificherò questa libreria. In ogni caso, suggerisco l'uso di spid-saml-check anziché di spid-testenv2.

matteo2411 commented 3 years ago

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 commented 3 years ago

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 https://github.com/italia/spid-testenv2/pull/327 e funzionerà.

matteo2411 commented 3 years ago

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 commented 3 years ago

a proposito del metadata generato in automatico, ho riscontrato che genera un solo . 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 che sono invece differenti per tutti gli IdP

lscorcia commented 3 years ago

@matteo2411 , anche il problema sul KeyName è un bug in spid-testenv2, questo però è già stato corretto: https://github.com/italia/spid-testenv2/pull/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 :)

matteo2411 commented 3 years ago

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

image

lscorcia commented 3 years ago

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 commented 3 years ago

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:

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

corretto?

lscorcia commented 3 years ago

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 commented 2 years ago

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 commented 2 years ago

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!

nicolabeghin commented 8 months ago

Chiudo in quanto mergiato in https://github.com/italia/spid-keycloak-provider/pull/30