italia/spid-spring

Elenco problematiche

vincenzocorso opened this issue · 2 comments

vincenzocorso commented

Salve, sto cercando di far funzionare questo SDK per testare l'autenticazione tramite spid.
Nel fare ciò ho riscontrato una serie di problematiche:

I problemi nella richiesta sono:

  1. L'elemento isPassive è presente. Stando alle regole tecniche spid l'elemento isPassive NON deve essere presente (ad indicare il valore false). Ad esempio lo spid test env mostra un errore (anche se il valore dell'elemento è posto a false). Sarebbe preferibile non inserirlo proprio.
  2. Sempre stando alle regole tecniche, nell’elemento <AuthnRequest> deve essere presente solo UNO dei seguenti attributi: AssertionConsumerServiceIndex, AssertionConsumerServiceURL, ProtocolBinding. La richiesta generata da questo SDK li mette tutti. Lo spid test env mostra quindi un errore, segnalando la cosa.
  3. L'attributoFormat dell'elemento <Issuer> deve essere uguale a urn:oasis:names:tc:SAML:2.0:nameid-format:entity. Viene posto invece a urn:oasis:names:tc:SAML:2.0:nameid-format:transient.
  4. Questo SDK genera una richiesta per il livello di autorizzazione L2, ma non aggiunge l'attributo ForceAuthn nell'elemento <AuthnRequest> (richiesto per i livelli L2 e L3). Sarebbe anche preferibile lasciare scegliere a chi utilizza la libreria quale livello spid utilizzare (invece di hardcodare la stringa https://www.spid.gov.it/SpidL2)
  5. Nell'elemento <RequestedAuthnContext>, l'attributo Comparison viene impostato di default ad exact. Sarebbe preferibile scegliere come valore di default minimum. Citando le regole tecniche: "L’Identity Provider ha facoltà di utilizzare per l’autenticazione un livello SPID più alto rispetto a quelli risultanti dall’indicazione del richiedente mediante l’attributo Comparison. Tale scelta non deve comportare un esito negativo della richiesta". Cioè bisogna permette l'accesso anche a chi ha uno spid di livello superiore (es L3).
  6. Gli elementi <SignatureValue> e <DigestValue> sono vuoti. La richiesta non viene quindi firmata correttamente.
  7. Se si segue la richiesta POST, si deve solamente codificare in base64.
  8. Nel costruttore della classe AuthenticationInfoExtractor, viene hardcodato il valore https://spid.lecce.it. Per quale motivo?
  9. ? All'interno dell'elemento <KeyInfo>, vengono inseriti entrambi gli elementi <KeyValue> e <X509Data>. In teoria questo non dovrebbe essere un problema in sè. Chi utilizza il test env potrebbe incontrare un eccezione. Per maggiori informazioni vedere italia/spid-testenv2#325 e italia/spid-keycloak-provider#7

Ecco le soluzioni:
Per i problemi 1 e 2:

public AuthnRequest buildAuthenticationRequest(String assertionConsumerServiceUrl, Integer assertionConsumerServiceIndex, String issuerId, String id, String destination) {
  [...]
  AuthnRequest authRequest = authRequestBuilder.buildObject(SAML2_PROTOCOL, "AuthnRequest", "samlp");
  //authRequest.setIsPassive(false); // soluzione al problema 1
  authRequest.setIssueInstant(issueInstant);
  //authRequest.setProtocolBinding(SAML2_POST_BINDING); // soluzione al problema 2
  //authRequest.setAssertionConsumerServiceURL(assertionConsumerServiceUrl); // soluzione al problema 2
  authRequest.setAssertionConsumerServiceIndex(assertionConsumerServiceIndex);
  [...]
}

Per il problema 3:

private Issuer buildIssuer(String issuerId) {
  IssuerBuilder issuerBuilder = new IssuerBuilder();
  Issuer issuer = issuerBuilder.buildObject();
  issuer.setNameQualifier(issuerId);
  issuer.setFormat("urn:oasis:names:tc:SAML:2.0:nameid-format:entity"); // soluzione al problema 3
  issuer.setValue(issuerId);
  return issuer;
}

Per il problema 5:

private RequestedAuthnContext buildRequestedAuthnContext() {
  [...]
  RequestedAuthnContextBuilder requestedAuthnContextBuilder = new RequestedAuthnContextBuilder();
  RequestedAuthnContext requestedAuthnContext = requestedAuthnContextBuilder.buildObject();
  requestedAuthnContext.setComparison(AuthnContextComparisonTypeEnumeration.MINIMUM); // non EXACT
  requestedAuthnContext.getAuthnContextClassRefs().add(authnContextClassRef);
  [...]
}

Per il problema 6 vedere #10
Per il problema 7 ho sostituito l'intero metodo con questo:

public String encodeAndPrintAuthnRequest(AuthnRequest authnRequest) throws IntegrationServiceException {
  String requestMessage = printAuthnRequest(authnRequest);
  requestMessage = requestMessage.replace("\n", "").replace("\r", "");
  String encodedRequestMessage = Base64.getEncoder().encodeToString(requestMessage.getBytes());
  return encodedRequestMessage;
}

Risolti questi problemi, si ha un eccezione NullPointerException quando si prova a decodificare la SAMLResponse.

@Override
public ResponseDecoded processAuthenticationResponse(
	final HttpServletRequest request, final HttpServletResponse response)
			throws IntegrationServiceException {

  // TODO: configurazione SAML
  SAMLConfig saml2Config = new SAMLConfig();

Sfortunatamente non ho trovato soluzione a questo problema. Sembra sia dovuto al fatto che l'oggetto saml2Config non viene correttamente configurato (l'autore ha lasciato un TODO). Quindi al momento, con questo SDK, non si può decodificare la risposta ricevuta dall'idp.

Aprò un issue (e non una pull request) perché si tratta solamente di soluzioni "temporanee" che ho trovato. Pertanto dovrebbero essere controllate. Spero che in un giorno non troppo lontano questa repository venga sistemata.

f-lombardo commented

Ciao @vincenzocorso , penso che potresti incominciare con una PR delle tue soluzioni provvisorie, poi i maintainer le valuteranno, non pensi?

  • 👍1
peppelinux commented

Ciao @vincenzocorso
ho unito alcune PR che risolvevano parte delle problematiche da te esposte, ti andrebbe di fare un test con l'attuale master branch e aggiornare questo thread?

Te ne sarei enormemente grato