search

italia / spid-dotnet-sdk

SPID authentication library for .NET
BSD 3-Clause "New" or "Revised" License
28 stars 15 forks source link

Refactor IdP Metadata and Configuration #9

Closed ncarandini closed 6 years ago

ncarandini commented 6 years ago

Ho modificato il modello dei dati dell'Identity Provider, e coerentemente refattorizzato sia la libreria che le due applicazioni web di test per due motivi:

  1. Modifica della proprietà ID che prima era una stringa arbitraria e ora sia come nome che come contenuto corrisponde al campo EntityID del file di metadata che descrive ciascun IdP.
  2. Aggiunta delle proprietà OrganizationName, OrganizationDisplayName e OrganizationUrl che si trovano nel file di metadati dell'IdP.
  3. Rinomina di altre proprietà col nome XML dei rispettivi campi sempre nel file di metadati dell'IdP.

Conseguente modifica del codice di costruzione del file Json di configurazione (idpConfigDataList.json) con l'aggiunta di tutte le informazioni presa dai vari file di metadati degli IdP ad oggi registrati su AgID

ncarandini commented 6 years ago

Se siete disponibili, @congiuluc e/o @mrcarbook potete fare review della PR?

ncarandini commented 6 years ago

Nella PR ho anche aggiunto una parametrizzazione del valore di AttributeConsumingServiceIndex che per errore è rimasta fino ad oggi fissata nel codice (e pari a 1, corrispondentemente al file di metadata creato durante l'hackathon per il sito di test di Poste Italiane) del metodo SamlHelper.BuildAuthnPostRequest. Ora la chiamata al metodo ha un parametro aggiuntivo che consente la libera impostazione di questo valore, com'è giusto che sia.

congiuluc commented 6 years ago

Ciao Nik, La configurazione di base è buona anche se rimane troppo focalizzata su SAML, considerando che in futuro SPID si potrà implementare anche con altri protocolli penserei di lasciare la configurazione dell'identity provider il più aperta possibile. Ho modificato il tuo branch cercando di rendere il tutto il più aperto possibile ed ho implementato anche il caricamento dinamico dei provider per bottoni SPID. Ho aggiunto un nuovo branch : idplist-lco https://github.com/italia/spid-dotnet-sdk/tree/ipdlist-lco Dagli un'occhiata e se per te va bene faccio la pull request ;) Saluti Luca

ncarandini commented 6 years ago

Ciao @congiuluc qui si chiedeva una review, non l'aggiunta di altre funzionalità o una modifica concettuale di quanto già esistente nell'attuale soluzione. Le modifiche sono banali, ho evitato di mergiare direttamente io perché è buona norma sottoporre a revisione il proprio lavoro anche se si è mantainer del progetto.

Se mi confermi che dal punto di vista operativo il codice è ok, io poi posso mergiare. Poi ben vengano tutte le proposte di generalizzazione (attenzione però agli antipatterns, C# è un linguaggio fortemente tipizzato), che potrai fare su una tua branch.

In generale non è il caso di fare branch di branch di altri. Ovvero, tutto è possibile in Git, ma ci sono dei modus operandi e una volta scelto uno è bene attenervisi. In questo progetto @alexrj ha ben definito che il metodo è il seguente: ciascuno fa una branch dal master, ci mette le sue modifiche e le propone con una PR. Li viene discussa e se accettata viene mergiata nel master.

In attesa della tua (se vorrai) code review, ti auguro un buon 2018!