Closed pokono closed 2 years ago
Ciao @pokono,
a mio parere l'avviso fa riferimento al metadata che gli SP comunicano agli IdP e questo non deve essere necessariamente generato da spid-laravel
o da altro software, ma può anche essere compilato manualmente secondo le specifiche.
Ciò detto, ritengo che per generare un metadata che sia rispondente alle specifiche dell'avviso 29 sia necessario aggiungere le informazioni richieste manipolando il contenuto XML gestito dalla funzione metadata
.
Certamente va aggiunta anche una funzionalità di validazione delle nuove informazioni inserite e i rispettivi test.
@pdavide Il processo è definitivamente fuorviante. Abbiamo un'applicazione in produzione, con tutti i requisiti tecnici approvati e non riusciamo a finalizzare per via del metadata. (Il quale era conforme ai requisiti al momento in cui abbiamo passato i requisiti tecnici.) È assurdo. Vorrei aggiungere un po' di "storia".
Utilizzando il link del metadata generato dalla libreria, questa è la risposta dell'AGID che abbiamo ricevuto:
Buon pomeriggio.
Il metadata da voi comunicato non è conforme con l’Avviso SPID n.29/2020, cui bisogna attenersi obbligatoriamente dal 15 settembre per tutti i nuovi metadata: mancano infatti del tutto i due tag <ContactPerson> previsti nell’Avviso.
Vi chiediamo di riformare il metadata conformemente con l’Avviso SPID n.29/2020 –almeno versione 1.0– segnalandovi sin da ora che, entro le scadenze previste dall’attuale versione dell’Avviso (versione 2.0) vi verrà inviato un nuovo certificato (afferente la stessa chiave privata) con il quale dovrete comunque generare un metadata conforme con l’Avviso SPID 29/2020 nella versione attuale (la 2.0).
Modificando il metadata a mano, abbiamo ricevuto la risposta che il metadata non va inviato a mano ma va usato l'url https dell'applicazione.
Suggerimenti?
@pokono il package in questo repository non è un prodotto di AGID ma un software che si evolve con il contributo della community, non è assolutamente obbligatorio utilizzarlo per il proprio Service Provider.
Come tu stesso avevi scritto nel tuo primo commento, la strada migliore è contribuire con una PR.
Rispetto all'urgenza delle vostre esigenze, potete tranquillamente esporre il metadata corretto su un URL non gestito da questo package oppure fare override della route definita nel package.
Ciao @pdavide, grazie per la dritta!
Nel frattempo, se può aiutare qualcuno, noi abbiamo creato un middleware che intercetta le risposte delle routes spid
e nel caso del metadata, lo modifichiamo come segue.
<?php
namespace App\Http\Middleware\Spid;
use Closure;
use Illuminate\Http\Request;
use Illuminate\Support\Str;
class MetadataModifier
{
/**
* Handle an incoming request.
*
* @param Request $request
* @param Closure $next
* @return mixed
*/
public function handle($request, Closure $next) {
$response = $next($request);
if ($request->path() == 'spid/metadata') {
$content = $response->getOriginalContent();
$content = Str::replaceFirst(
'xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata"',
'xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata" xmlns:spid="https://spid.gov.it/saml-extensions"',
$content
);
$content = Str::replaceLast(
'</md:EntityDescriptor>',
'<md:ContactPerson contactType="other">
<md:Extensions>
<spid:VATNumber></spid:VATNumber>
<spid:FiscalCode></spid:FiscalCode>
<spid:Private/>
</md:Extensions>
<md:EmailAddress></md:EmailAddress>
<md:TelephoneNumber></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></fpa:IdCodice>
</fpa:IdFiscaleIVA>
<fpa:Anagrafica>
<fpa:Denominazione></fpa:Denominazione>
</fpa:Anagrafica>
</fpa:DatiAnagrafici>
<fpa:Sede>
<fpa:Indirizzo></fpa:Indirizzo>
<fpa:NumeroCivico></fpa:NumeroCivico>
<fpa:CAP></fpa:CAP>
<fpa:Comune></fpa:Comune>
<fpa:Provincia></fpa:Provincia>
<fpa:Nazione>IT</fpa:Nazione>
</fpa:Sede>
</fpa:CessionarioCommittente>
</md:Extensions>
<md:Company></md:Company>
<md:EmailAddress></md:EmailAddress>
<md:TelephoneNumber></md:TelephoneNumber>
</md:ContactPerson>
</md:EntityDescriptor>',
$content
);
$response->setContent($content);
}
return $response;
}
}
Appena ho un attimo di tempo farò una PR così lo sistemiamo ;)
@pokono grazie mille per il contributo! Nella futura PR consiglierei un approccio basato sulla strutturazione del dato in formato XML piuttosto che uno basato sulla manipolazione delle stringhe. Sarebbe davvero utile anche avere un test di validazione del metadata finale basato su XSD.
Si! Assolutamente, pensavo di aggiungere i parametri necessari direttamente nella configurazione e l'xml lo generiamo.
Sto' generando il metadata per un ente, e quindi si e' creata la necessita' di generare la sezione contact person tipo questa:
<md:ContactPerson contactType="other">
<md:Extensions>
<spid:IPACode>c_a123</spid:IPACode>
<spid:Public/>
</md:Extensions>
<md:EmailAddress>info@comune.roccacannuccia.xx.it</md:EmailAddress>
</md:ContactPerson>
ho modificato la Settings.php e la Metadata.php: Settings.php
@@ -651,7 +651,7 @@ class Settings
}
foreach ($settings['contactPerson'] as $type => $contact) {
- if (!isset($contact['givenName']) || empty($contact['givenName'])
+ if (!isset($contact['extensions']) || empty($contact['extensions'])
|| !isset($contact['emailAddress']) || empty($contact['emailAddress'])
) {
$errors[] = 'contact_not_enought_data';
Metadata.php
@@ -107,13 +107,26 @@ ORGANIZATIONSTR;
$strContacts = '';
if (!empty($contacts)) {
$contactsInfo = array();
- foreach ($contacts as $type => $info) {
- $contactsInfo[] = <<<CONTACT
+ foreach ($contacts as $type => $info) {
+ $strContacts = <<<CONTACT
<md:ContactPerson contactType="{$type}">
- <md:GivenName>{$info['givenName']}</md:GivenName>
- <md:EmailAddress>{$info['emailAddress']}</md:EmailAddress>
+ <md:Extensions>
+
+CONTACT;
+ foreach ($info['extensions'] as $exte => $valore) {
+ if (empty($valore)) {
+ $strContacts .= " <spid:".ucfirst($exte)."/>\n";
+ }
+ else {
+ $strContacts .= " <spid:".ucfirst($exte).">{$valore}</spid:".ucfirst($exte).">\n";
+ }
+ }
+ $strContacts .= <<<CONTACT
+ </md:Extensions>
+ <md:EmailAddress>{$info['emailAddress']}</md:EmailAddress>
</md:ContactPerson>
CONTACT;
+ $contactsInfo[] = $strContacts;
}
$strContacts = "\n".implode("\n", $contactsInfo);
}
@@ -175,7 +188,7 @@ METADATA_TEMPLATE;
$isDefaultAcs = (0 === $sp['assertionConsumerService']['index']) ? 'isDefault="true"' : '';
$metadata = <<<METADATA_TEMPLATE
<?xml version="1.0"?>
-<md:EntityDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata"
+<md:EntityDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata" xmlns:spid="https://spid.gov.it/saml-extensions"
entityID="{$spEntityId}">
<md:SPSSODescriptor AuthnRequestsSigned="{$strAuthnsign}" WantAssertionsSigned="{$strWsign}" protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
{$sls} <md:NameIDFormat>{$sp['NameIDFormat']}</md:NameIDFormat>
infine in in spid-saml.php (vendor/italia/spid-laravel/config/ ho aggiunto in fondo:
'contactPerson' => [
'other' => [
'extensions' => [
'iPACode' => 'c_a123',
'public' => '',
],
'emailAddress' => 'info@comune.rocacannuccia.xx.it',
],
],
c'e da miglirare il tutto ed implementare la sezione billing e per i privati. Eventualmente per le PR se puo' provvedere qualcuno piu' esperto. Scusate orrori, errori ed omissioni.
La soluzione esposta da @pokono non credo che funzioni da un punto di vista di validazione. Se non erro il metadata contiene una firma e con un middleware vai a modificare il contenuto dopo che questo è stato firmato, sbaglio? Il controllo della firma andrebbe a fallire.
La soluzione esposta da @pokono non credo che funzioni da un punto di vista di validazione. Se non erro il metadata contiene una firma e con un middleware vai a modificare il contenuto dopo che questo è stato firmato, sbaglio? Il controllo della firma andrebbe a fallire.
Ciao, ti confermo che sono in produzione da 1 anno e sta andando tutto bene. Il motivo per il quale funziona è che questo metadata è usato solo per scambiare informazioni tra sistemi, non va a modificare l'xml che viene scambiato durante le richieste di autenticazione.
Ovviamente, la mia soluzione non è bella, era solo una cosa temporanea finché io o altri abbiamo tempo per sistemare la libreria così da generarlo dinamicamente.
@pdavide come mai questo issue non è stato risolto? Sembra evidente che il metadata generato non è conforme, questo rende il pacchetto inutilizzabile se non con soluzioni come quella di @pokono Vorrei capire se c'è un motivo dietro questa scelta di non integrare il fix
@pdavide come mai questo issue non è stato risolto? Sembra evidente che il metadata generato non è conforme, questo rende il pacchetto inutilizzabile se non con soluzioni come quella di @pokono Vorrei capire se c'è un motivo dietro questa scelta di non integrare il fix
Ciao @cod3rshotout, se hai un fix pronto o in lavorazione per risolvere questa issue, potresti fare una PR?
@pdavide come mai questo issue non è stato risolto? Sembra evidente che il metadata generato non è conforme, questo rende il pacchetto inutilizzabile se non con soluzioni come quella di @pokono Vorrei capire se c'è un motivo dietro questa scelta di non integrare il fix
Ciao @cod3rshotout, se hai un fix pronto o in lavorazione per risolvere questa issue, potresti fare una PR?
Ciao @pdavide, potrei integrare ciò che ha suggerito @pokono con una PR ma la mia domanda precedente è questa: come mai non è stata inserita la sua soluzione? Anche perché questo issue è attivo dal 2020 ed un peccato non aggiornare questa libreria. Non voglio fartene assolutamente una critica, anzi, ti ringrazio per il lavoro svolto. Vorrei solo capire perché non la si è integrata, comunque in giornata invio una PR con le modifiche suggerite da @pokono
Ciao @pdavide, potrei integrare ciò che ha suggerito @pokono con una PR ma la mia domanda precedente è questa: come mai non è stata inserita la sua soluzione? Anche perché questo issue è attivo dal 2020 ed un peccato non aggiornare questa libreria. Non voglio fartene assolutamente una critica, anzi, ti ringrazio per il lavoro svolto. Vorrei solo capire perché non la si è integrata, comunque in giornata invio una PR con le modifiche suggerite da @pokono
@cod3rshotout la soluzione proposta da @pokono non è stata integrata per 3 motivi:
La soluzione proposta da @chech66 affronta il problema in fase di generazione del contenuto del metadata, tuttavia eviterei di appesantire la patch se non è strettamente necessario.
L'approccio che ritengo più adatto è integrare il metodo metadata()
aggiungendo al contenuto generato da php-saml ciò che è necessario lavorando con XML (e non con le stringhe). In questo modo si può anche validare la configurazione e restituire all'utilizzatore della libreria informazioni utili su eventuali errori di configurazione.
@pdavide ho capito, potrei provare ad integrare una possibile soluzione con il metodo da te proposto. Tuttavia, non conoscendo a pieno la tua libreria, né quella di spid-php, sarei felice se potessi integrare tu questo fix, quanto meno per generare un metadata corretto.
Ciao sto provando a scrivere una patch con l'approccio che suggerisce @pdavide ma ho questo problema. Ho aggiornato i file xsd con quelli ufficiali, tra i quali ad esempio spid-invoicing.xsd che contiene le "extensions" per la parte billing (mi serve perché noi siamo dei privati). Siccome giustamente vorrei lasciare il test attuale che valida il metadata con xsd, sono costretto a creare una nuova patch di OneLogin/Metadata per modificare la dichiarazione dei namespace nell'header xml. Quando cerco di farlo però, la libreria sembra esplodere, se cerco di stampare l'xml risultato mi esce un errore di out of memory. A qualcuno è già capitato? grazie
sono costretto a creare una nuova patch di OneLogin/Metadata per modificare la dichiarazione dei namespace nell'header xml.
Ciao @nicofari, non si riesce a modificare il metadata dentro il metodo metadata() invece di creare una nuova patch?
ciao @pdavide certo il metadata lo modifico lì (ho seguito le tue indicazioni) però non toccavo l'intestazione, facevo solo delle aggiunte tu dici, già che ci siamo, di modificare anche quelle lì e buonanotte?
@nicofari si direi di evitare le patch finché è possibile.
Ho fatto una PR, ma ci sono dei test d'integrazione che falliscono. Non capisco perché, però. Il primo, nei dettagli, parla di << Detected deprecations in use:
.php_cs.dist
is deprecated, rename to .php-cs-fixer.dist.php
.
Ma non c'è, nella mia PR, quel file lì
ah ma sembra che si lamenti un tool per il code style.. devo ammettere che non lo conosco anche se ho visto dei warning di PhpStorm, relativi a questo Php-cs-fixer, che mi sono affrettato a ignorare ...
Ciao!
Rivedendo la nuova versione dell'avviso 29 v2, ho notato che ci sono dei parametri aggiuntivi nel metadata che non sembrano essere disponibili dalla configurazione.
Nello specifico mi riferisco ai alla sezione
ContactPerson
.L'avviso è disponibile al seguente link: https://www.agid.gov.it/sites/default/files/repository_files/spid-avviso-n29v2-specifiche_sp_pubblici_e_privati.pdf
La situazione è abbastanza urgente visto che la compatibilità deve essere in produzione entro Novembre 14, 2020 ed il nostro sistema è attualmente in produzione.
Se mi date 2 dritte posso anche iniziare una PR.
Grazie mille, Ivan