italia / spid-keycloak-provider

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

Inconsistenza test "response" eseguiti con lo "spid-saml-check" #12

Closed erapsag closed 2 years ago

erapsag commented 3 years ago

Ciao Luca, ho un problema con lo "spid-saml-check" (tirato su in locale), ed in particolare con i test sulla "Response". I test sul "Metadata SP" e sulla "Request" vengono infatti passati tutti con successo (apparte l'ultimo test di "Request/CheckStrict" https://github.com/lscorcia/keycloak-spid-provider/issues/8 che anche altri utenti riscontrano), mentre quelli riguardanti il capitolo "Response" non sempre portano ad un risultato coincidente con quello atteso. Ad esempio nel caso di "01. Response corretta" va tutto bene (login ok), così come nel caso di "02. Response non firmata" (login errore), e così come in tanti altri casi, ma ci sono altri test, come per esempio "08. ID non specificato" o "13. Response - Formato IssueInstant non corretto", che invece di andare in errore terminano con una corretta procedura di login all'interno del nostro applicativo. Ora i test da effettuare sono un centinaio e non ho avuto modo di testarli tutti.

Volevo chiedere se qualcuno ci era già passato, se aveva effettuato tutti i test manualmente, e se avevate trovato corrispondenza tra risultato atteso ed esito del test per ognuno di essi.

Inoltre sapete se è possibile effettuare i test anche con lo spid-validator indicato da agid nella documentazione, quello che indica di inserire nel bottone in fase di verifica per l'accreditamento (https://validator.spid.gov.it/metadata.xml)?

lscorcia commented 3 years ago

Ciao @erapsag , come hai potuto notare ti confermo che non ho testato tutti i 100 e passa controlli previsti dallo spid-saml-check. In realtà la maggior parte di questi sono controlli previsti dallo standard SAML, quindi mi aspetterei che fosse Keycloak stesso a rifiutarli, senza delegare a questo plugin. Ci vorrà del tempo. Proverò a compilare una tabella sul wiki con l'elenco i test superati e non superati, almeno per avere una traccia.

Per quanto riguarda lo spid validator online, a quanto ho capito dovrebbe essere riservato agli operatori AGID, ma è lo stesso tool che si avvia via docker quindi non c'è motivo di usarlo per controlli aggiuntivi.

erapsag commented 3 years ago

ciao @lscorcia, grazie della risposta. Più che altro volevo capire se potesse essere un problema solo mio legato magari a qualche configurazione da approfondire, ma effettivamente sembrerebbe più una questione relativa a Keycloak.

Comunque se non sbaglio delle soluzioni che utilizzano Keycloak con il tuo provider hanno passato l'accreditamento AgID?

nicolabeghin commented 3 years ago

Se può tornare utile ho reso disponibile online lo SPID SAML checker dopo l'ennesima volta che lo ri-buildavo in locale dal repository ufficiale.

nicolabeghin commented 3 years ago

@erapsag se può tornare utile unire le forze, sto affrontando il medesimo problema dei 100+ check. ad esempio sto affrontando https://github.com/lscorcia/keycloak-spid-provider/issues/18 (al momento bloccato perchè non riesco a recuperare un riferimento alla request e conseguentemente validare quanto richiesto). è veramente un inferno.

erapsag commented 3 years ago

ciao @nicolabeghin, anche io qualche giorno fa ho fatto un pò di test modificando le classi Keycloak che gestiscono le Response SAML, verificando che effettivamente in questo modo si possono aggiungere i controlli che mancano e richiesti da AgID. Ho implementato solo alcuni controlli a scopo di verifica (ad es. controllo se l'attributo obbligatorio è vuoto, quelli più semplici in pratica). per poi lasciare in stand-by la cosa, penso la riprenderò fra qualche giorno. A quanto ho capito tu stai procedendo modificando il provider e non direttamente Keycloak, dico bene?

Magari possiamo vedere anche che ne pensa @lscorcia, e se per caso anche lui ha dato un occhiata alla questione o ci sta lavorando.

lscorcia commented 3 years ago

Ciao, sto seguendo le vostre implementazioni ma in questi giorni purtroppo devo consegnare altro, ci vorrà qualche altro giorno temo prima che possa riprendere il filo. L'idea che ho sempre seguito comunque è che se i problemi riguardano SAML le correzioni vanno inviate a Keycloak, e mi sembra che per buona parte dei controlli SPID sia proprio così. Ho inviato un post sulla ML keycloak-dev: https://groups.google.com/d/msgid/keycloak-dev/8c464a26-1582-4abf-aad4-98f559b9b341n%40googlegroups.com?utm_medium=email&utm_source=footer , purtroppo però non ho avuto risposta.

Credo che l'approccio più solido sia creare una piccola tabella su un google docs dei controlli SPID, tabulare cosa viene superato e cosa no e se è un controllo built-in di SPID o è previsto da SAML. I primi vanno corretti nel plugin, per i secondi va preparata una PR per Keycloak, purtroppo però per ogni PR vogliono i test automatici e sono quelli che richiedono più lavoro...

nicolabeghin commented 3 years ago

grazie @erapsag e @lscorcia , il solo fatto di sapere di non essere da soli rincuora :D

@lscorcia per caso hai un'indicazione sull'approccio suggerito per avere in canna la Request SAML nel momento dell'evaluation della Response? considerato quanto sopra vorrei cercare di inserire nel provider quantomeno i check basati sulla Request (es. Request.ID matching Response.InResponseTo) senza toccare il core Keycloak ma non voglio credere che non ci sia già a standard lo store della request..!

peppius commented 3 years ago

Ciao @nicolabeghin @lscorcia @erapsag , abbiamo inviato il metadata creato con il provider ad AGID per la validazione, ma già sottolineato da voi, falliscono molti test sulla "Response". Avremo intenzione di completarli tutti per ri-sottomettere la richiesta di validazione, ne avete già implementato qualcuno o avete indicazioni su come completare i test sulla "Response" ? Li possiamo inserire sul provider o bisogna modificare il core di keycloak?

Grazie

nicolabeghin commented 3 years ago

@peppius noi abbiamo cominciato a chiuderne alcuni: vedi PR #15 e #18 e #23 - servirebbe una mano per approcciare IL MOSTRO. Di seguito i check approntati - alcuni sono banali onestamente, si tratta solo di avere voglia di mettersi.

spid validation checks for anomalia 16, 17, 18, 41, 42, 51, 52, 53, 54, 55, 56, 57, 60, 61, 62

lscorcia commented 3 years ago

Ho aggiunto il codice per la verifica dei controlli implementati da @nicolabeghin (grazie ancora!). Purtroppo non ho ancora avuto tempo di preparare la tabella sul wiki ma con l'approccio che ho utilizzato è banale aggiungere i controlli mancanti. Sarebbe utile sapere quali controlli mancano per superare la validazione di AGID, io purtroppo ho già superato il primo accreditamento, quindi ci testano solo il metadato. Se avete una lista di controlli ancora non superati con la release 1.0.8 fatemi sapere!

lscorcia commented 3 years ago

Aggiunta documentazione per i test verificati/ancora da verificare nel wiki all'indirizzo https://github.com/lscorcia/keycloak-spid-provider/wiki/Compliance-tests-results . Tenetemi informato se ricevete esiti di test o se avete provato altri di quelli che non sono ancora riuscito a testare personalmente. Grazie!

zenor70 commented 2 years ago

Buongiorno. Ho effettuato tutti i test di validazione di Request e Response. In breve, perché la mia ditta possa diventare soggetto aggregatore, sto cercando di configurare spid-keycloak-provider versione 1.0.11 (non è l'ultima, lo so) su Keycloak 16.1.0: sto riscontrando pure io diverse problematiche. Ecco il riepilogo: Request: tutto ok: Response: una serie di problemi Allego screenshot e pdf con il dettaglio delle validazioni lato Response. Vi ringrazio per il supporto.

image image image

Il nostro IDM è raggiungibile con indirizzo pubblico: se vi è utile, posso fornirvi via slack indirizzo e metadati. Ho effettuato dei test sia con https://demo.spid.gov.it/validator che con https://www.spid-validator.it.

Dettaglio: 2022-03-04_SPID Validator.pdf

marcello-travaglini commented 2 years ago

Ciao @lscorcia ho creato un branche in cui ho inserito tutti i controlli sulla response, se volete testarlo è a disposizione su:

https://github.com/marcello-travaglini/spid-keycloak-provider/tree/spid-response-check

Ho anche inserito un pulsante per attivare il debug della response SAML nella configurazione del provider. Se il debug è attivo viene mostrato a video il dettaglio del check non superato. Se il debug non è attivo viene mostrato un messaggio di errore generico, come da specifiche AGID.

Domani cercherò di fare ulteriori di test perchè entro il fine settimana devo risottomettere l'accreditamento ad AGID.

lscorcia commented 2 years ago

Grazie mille @marcello-travaglini ! Riesci ad aprirmi una pull request, così mi viene più facile verificare e mergeare le modifiche?

marcello-travaglini commented 2 years ago

Ciao @lscorcia, non so se hai avuto modo di esaminare le mie modifiche. Penso che ci sia un problema con il controllo del EntityId del Idp.

Guardando il codice mi sembra che tu abbia utilizzato EntityId SAML di Keycloak per valorizzare la proprietà dei metadata.xml

Per uno dei controlli avevo dedotto l'EntityId dalla url di login del provider, ma non tutti hanno un formato corretto. Per poter effettuare correttamente il controllo sto aggiungendo un nuovo campo EntityIdIdp nella configurazione di Keycloak in modo tale da poter effettuare correttamente il controllo. Ovviamente questo campo dovrà essere inserito manualmente a meno che non si modifichi anche la procedura di acquisizione del metatada.xml

lscorcia commented 2 years ago

Ciao, per motivi personali mi ci vorrà un po' per poter vedere questa PR (sicuramente non prima della fine della settimana prossima). A prima vista comunque mi sembra un ottimo lavoro!

Mi pare di capire che nella seconda parte della issue tu ti riferisca al controllo che l'entity ID dell'IdP coincida con quanto atteso. Credo che tu abbia ragione, anzi, sarebbe opportuno proporre questa funzionalità direttamente in Keycloak oltre che aggiungerla qui - e si, bisognerebbe anche precaricare il dato dal metadato.

marcello-travaglini commented 2 years ago

Volevo comunicarvi che ho appena concluso, correttamente, la procedura tecnica SPID.

Grazie per il lavoro che avete fatto.

nicolabeghin commented 2 years ago

Volevo comunicarvi che ho appena concluso, correttamente, la procedura tecnica SPID

Ottimo, bellissima notizia per avere Keycloak compatibile SPID al 100%!!

peppelinux commented 2 years ago

Bravo @marcello-travaglini

lscorcia commented 2 years ago

Thanks @marcello-travaglini , your contributions have been merged!