Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1_SimpleSPWebApp] KeyNotFoundException LOGOUT #41

Open
Eduardo1977 opened this issue Jan 22, 2025 · 20 comments · May be fixed by #42
Open

[1_SimpleSPWebApp] KeyNotFoundException LOGOUT #41

Eduardo1977 opened this issue Jan 22, 2025 · 20 comments · May be fixed by #42

Comments

@Eduardo1977
Copy link

Ho effettuato con successo la federazione in ambiente di preproduzione, utilizzando l'ultima versione 2.0.2.
Il login con la carta di test funziona e restituisce correttamente i dati ma se effettuo il logout va in errore :

KeyNotFoundException
The given key 'revocation_endpoint' was not present in the dictionary.

Ho scordato qualcosa nell'appsettings?

Il primo indiziato potrebbe essere "RedirectUris" ?
Grazie

@Eduardo1977 Eduardo1977 changed the title [1_SimpleSPWebApp] KeyNotFoundException [1_SimpleSPWebApp] KeyNotFoundException LOGOUT Jan 23, 2025
@Eduardo1977
Copy link
Author

Aggiungo che dopo che passa un pò di tempo, il logout invece viene eseguito senza errore.

P.S. Ho provato a mettere false a "LongSessionsEnabled" ma il logout da sempre l'errore
KeyNotFoundException
The given key 'revocation_endpoint' was not present in the dictionary

@PiemP
Copy link
Contributor

PiemP commented Jan 23, 2025

Il codice presenta una problematica in quel punto. La cosa che capisco io (non sono così esperto per cui prendete la cosa con le pinze) da quello che vedo nella EC esposta da https://preprod.oidc.registry.servizicie.interno.gov.it è che non hanno un revocation ednpoint: il ché si traduce nel fatto che viene sempre lanciata un eccezzione ... anche sistemando il codice.

@PiemP
Copy link
Contributor

PiemP commented Jan 23, 2025

@Eduardo1977 riesci a testarmi la modifica del #42 . Grazie.

@Eduardo1977
Copy link
Author

Eduardo1977 commented Jan 23, 2025

@PiemP Si grazie, dovrei riuscire a pomeriggio

Intanto, stavo leggendo la documentazione SPIDCIE OIDC a tal riguardo
https://docs.italia.it/italia/spid/spid-cie-oidc-docs/it/versione-corrente/logout.html

https://docs.italia.it/italia/spid/spid-cie-oidc-docs/it/versione-corrente/revocation_endpoint.html#revocation-endpoint

@Eduardo1977
Copy link
Author

Ciao @PiemP , il logout adesso viene eseguito correttamente.
Ho fatto vari cicli di login/logout: non viene sollevata più nessuna eccezione .
Ho provato anche con browser diversi: Edge e Chrome
Grazie

@PiemP
Copy link
Contributor

PiemP commented Jan 23, 2025

Grazie a te @Eduardo1977 ...

non sono così esperto per cui prendete la cosa con le pinze) da quello che vedo nella EC esposta da https://preprod.oidc.registry.servizicie.interno.gov.it/ è che non hanno un revocation ednpoint

Rettifico questa cosa... oddio più che rettifico ho sbagliato il soggetto... ho recuperato l'EC di https://preproduzione.oidc.idserver.servizicie.interno.gov.it e il revocation endpoint è effettivamente presente ... faccio un paio di verifiche e aggiungo eventualmente altre modifiche al PR.

Ho fatto vari cicli di login/logout: non viene sollevata più nessuna eccezione .

attualmente mi limito a fare un log di livello warning quando un revocation endpoint non è definito: per cui niente eccezzione. Però, dato quanto emerso dalle verifiche su https://preproduzione.oidc.idserver.servizicie.interno.gov.it, è meglio che verifichi effettivamente cosa succede di modo che l'informazione venga recuperata correttamente.

Eventualmente ti chiederò di fare altri test 😉.

@Eduardo1977
Copy link
Author

OK. Prima non capivo perchè avessi indicato il registry, che in effetti non ha un revocation endpoint.
Invece, https://preproduzione.oidc.idserver.servizicie.interno.gov.it/.well-known/openid-federation

ha questo:
"revocation_endpoint": "https://preproduzione.oidc.idserver.servizicie.interno.gov.it/idp/profile/oauth2/revocation"

@Eduardo1977
Copy link
Author

Eduardo1977 commented Jan 23, 2025

Ciao @PiemP , con la versione 1.2.0 non avevo mai riscontrato il problema del logout.
Hai provato a confrontare le due versioni di "private async Task RevokeToken(string accessToken)" ?
Ci sono due differenze, in particolare la prima :

V2.0.2
var certificate = rp!.OpenIdCoreCertificates!.FirstOrDefault(occ => occ.KeyUsage == Enums.KeyUsageTypes.Signature)!;

V1.2.0
var certificate = rp!.OpenIdCoreCertificates!.FirstOrDefault()!;

Però, non sembra essere responsabile dell'eccezione che viene sollevata dal codice subito dopo (e che hai tolto con il fix):

var revocationEndpoint = idp!.EntityConfiguration.Metadata.OpenIdProvider!.AdditionalData[SpidCieConst.RevocationEndpoint] as string;
Throw.If(string.IsNullOrWhiteSpace(revocationEndpoint),
$"No RevocationEndpoint specified in the EntityConfiguration of the IdentityProvider {issuer}");

@PiemP
Copy link
Contributor

PiemP commented Jan 23, 2025

Il problema è un altro ed è strano ... ho un idea per risolverlo ma mi piacerebbe capirlo fino in fondo ... praticamente l'oggetto OpenIdConnectConfiguration (di Microsoft) ha una proprietà AdditionalData dove dovrebbe finirci tutto quello che è accessorio rispetto ad una configurazione standard. Adesso, non ho capito perché, ma pare che questa proprietà "non funzioni" più: credo a causa di un aggiornamento alle librerie che ha introdotto questo bug ma non capisco se è qualcosa relativo all'attributo JsonExtensionData o a qualcos'altro.

Io sono su .net 8.0 e riesco a replicare (credo, perché l'implementazione che uso io non richiama SignOutAsync) la tua anomalia in quanto vedo la proprietà AdditionalData vuota e quindi non ho il revocation_endpoint.

Ho provato a giochicchiare con le versioni dei pacchetti ma non riesco a vedere la proprietà compilata per cui sarei per creare una versione custom di OpenIdConnectConfiguration per avere le proprietà custom che vengono utilizzate nella libreria: tra cui il revocation_endpoint. Di modo da evitare eventuali anomalie di questo tipo.

Sarebbe bello approfondire ma al momento non ho tempo.

@PiemP
Copy link
Contributor

PiemP commented Jan 24, 2025

Con questo commit 01be195 ho introdotto la mia idea "semplice" per la soluzione dell'anomalia della mancata compilazione del campo AdditionalData di OpenIdConnectConfiguration con il commit successivo vado ad introdurre una soluzione più compliant.

@Eduardo1977
Copy link
Author

Eduardo1977 commented Jan 24, 2025

Come avevo scritto all'inizio, non mi è chiaro però, perchè dopo un pò di tempo, il logout veniva eseguito correttamente senza sollevare nessuna eccezione. @PiemP avevi fatto caso a quest'altro comportamento che avevo riscontrato?
Stamattina ho aperto una nuova issue su un dubbio, che avevo fin dall'inizio, #43

Il problema del logout potrebbe dipendere in qualche modo dalla configurazione dei certificati che ho effettuato?

@PiemP
Copy link
Contributor

PiemP commented Jan 24, 2025

Aggiungo che dopo che passa un pò di tempo, il logout invece viene eseguito senza errore.

se ti riferisci a questo credo (e sottolineo credo, aka non sono sicuro al 100% perché non ho testato direttamente la call di SignOutAsync) che sia dovuto al fatto che la revoca del token avviene solo se esiste un access token valido vedi qui:

public override async Task SignOutAsync(AuthenticationProperties? properties)
{
var accessToken = await Context.GetTokenAsync(Options.SignOutScheme, OpenIdConnectParameterNames.AccessToken);
if (!string.IsNullOrWhiteSpace(accessToken))
{
await RevokeToken(accessToken);
}
await Context.SignOutAsync(Options.SignOutScheme);
Response.Redirect(properties?.RedirectUri ?? Options.SignedOutRedirectUri);
}

Hai fatto bene ad evidenziare la cosa.

@Eduardo1977
Copy link
Author

Ok grazie. Avevo scritto anche che con la v1.2.0 non si era mai verificato questo problema.
Inoltre, avevo notato anche delle lievi differenze nel "SpidCieConst.cs".

public const string RevocationEndpoint = "revocation_endpoint";

public const string CallbackPath = "/signin-spidcie";

public const string SignedOutCallbackPath = "/signout-callback-spidcie";

@PiemP
Copy link
Contributor

PiemP commented Jan 24, 2025

Avevo scritto anche che con la v1.2.0 non si era mai verificato questo problema. ...

Si ho letto, ma non conosco il codice della v1.2.0 (come non conosco bene questo codice) e non conosco le particolarità di quella versione ... per quello che ho potuto vedere ci sono nomi di classi differenti e usano librerie differenti (che potrebbero essere alla base del problema) ma la sostanza é quella ... posso solo dirti che sembra molto probabile che il problema si verficava perché non c'era il revocation endpoint dentro AdditionalData.

Nell'ultimo commit ho aggiunto un test e modificato un componente nei test per fare in modo che usi sempre lo stesso codice per il recupero dell'EC degli OP (lato test e lato applicativo) per verificare che il revocation endpoint sia compilato effettivamente. Il fix parte dall'assunto che nei test veniva utilizzato OpenIdConnectConfiguration.Create(...) per ottenere un oggetto OpenIdConnectConfiguration. Create utilizza un suo serializer specifico che sembra non dare i problemi che ho potuto constatare.

Rimane solo da testare l'ultimo commit. Io vedo la mia implementazione funzionante (e vedo il revocation endpoint negli AdditionalData) ma non chiamo la funzione di SingOutAsync di SpidCieHandler e non posso confermare il funzionamento al 100%.

@Eduardo1977
Copy link
Author

Ciao @PiemP, ho fatto il tentativo di assegnare direttamente il valore dell'endpoint dentro al task RevokeToken:
var revocationEndpoint = "https://preproduzione.oidc.idserver.servizicie.interno.gov.it/idp/profile/oauth2/revocation";

Però, sto riscontrando che fallisce comunque la chiamata per la revoca del token.
In particolare, viene sollevata una NullReferenceException dentro a "DecodeJoseResponse" in CustomHttpClientHandler:

Image

La request che viene inviata è questa:

{"Token":"eyJraWQiOiJkZWZhdWx0UlNBU2l ... ",
"TokenTypeHint":null,
"Address":"https://preproduzione.oidc.idserver.servizicie.interno.gov.it/idp/profile/oauth2/revocation",
"ClientId":"https://spidcieoidc ... ",
"ClientSecret":null,
"ClientAssertion":{"Type":"urn:ietf:params:oauth:client-assertion-type:jwt-bearer",
"Value":"eyJraWQiOiJDRjgwODREOEU3ND ... "},
"ClientCredentialStyle":1,
"AuthorizationHeaderStyle":0,
"DPoPProofToken":null,
"Parameters":[],
"Version":"1.1",
"VersionPolicy":0,
"Content":null,
"Method":{"Method":"GET"},
"RequestUri":null,
"Headers":[{"Key":"Accept",
"Value":["application/json"]}],
"Properties":{},
"Options":{}}

Non so se può dipendere da qualcuno di quei valori null ?
Secondo te la request ha qualcosa di errato?

@PiemP
Copy link
Contributor

PiemP commented Jan 30, 2025

Non so se può dipendere da qualcuno di quei valori null ?
Secondo te la request ha qualcosa di errato?

Probabile, così alla veloce non saprei dirti cosa non va. Mi servirebbe lo stack trace ...

Ci sono delle cose che non mi tornano ad occhio ... tipo che la richiesta è fatta in GET mentre in teoria dovrebbe essere un POST ... per il resto i dati ci sono tutti ... forse troppi. La risposta dovrebbe essere un HTTP 200 senza nessun dato.

Devo fare in modo che i miei servizi richiamino SignOutAsync di modo che posso capire anche io che succede e posso debuggare la cosa.

Dopo gli ultimi aggiornamenti nella PR #42 i dati per effettuare il revoke dovrebbero essere recuperati correttamente (anzi mi farebbe comodo se mi testassi la cosa visto che i miei servizi non chiamano ancora SingleLogoutAsync) ... è chiaro che finché i maintainer non approvano la PR non posso assicurarti che sia la soluzione giusta: per funzionare funziona... rimane in sospeso la questione relativa al fatto che la Deserialization di OpenIdConnectConfiguration si è rotta: nella #42 la cosa è stata risolta con un escamotage ma mi manca un giustificativo, qualcosa che confermi quanto ho visto a livello di codice durante il debug e mi faccia capire per bene come mai succede questa cosa e se è possibile risolverla senza il mio escamotage ... al momento non ho avuto ancora tempo di fare altre ricerche.

@Eduardo1977
Copy link
Author

Ciao @PiemP grazie per il riscontro. Per i test vedo se riesco la settimana prossima.

Vorrei chiederti se puoi darmi qualche chiarimento sull'utilizzo di certificati distinti per "OpenIdCoreCertificates" e "OpenIdFederationCertificates", perchè le istruzioni mi sembrano poche e ho anche il dubbio che ci sia un bug nella libreria.
Avevo aperto una issue #43

@Eduardo1977
Copy link
Author

Eduardo1977 commented Jan 31, 2025

Image

La request per revocare il token viene inviata da RevokeTokenAsync

var responseMessage = await Backchannel.RevokeTokenAsync(request);

che dovrebbe essere in POST come si vede nello screenshot

@Eduardo1977
Copy link
Author

Image

Stamattina ho approfondito meglio, la richiesta di revoca del token funziona e restituisce HTTP 200 !
L'eccezione che viene sollevata NullReferenceException dentro a "DecodeJoseResponse" è dovuta a Headers

{"Version":"1.1",
"Content":{"Headers":[{"Key":"Content-Length",
"Value":["0"]}]},
"StatusCode":200,
"ReasonPhrase":"200",
"Headers":[{"Key":"Set-Cookie",
"Value":["Pecorell@X=A0t+QpD...Aeu+w2EE/GKw$$",
"JSESSIONID=F15......63249223330AD761.1; Domain=.preproduzione.oidc.idserver.servizicie.interno.gov.it; Expires=Mon,
03 Feb 2025 10:47:27 GMT; Path=/; Secure; HttpOnly",
"ROUTEID=sticky.uno;domain=preproduzione.oidc.idserver.servizicie.interno.gov.it;path=/;"]},
{"Key":"Date",
"Value":["Mon,
03 Feb 2025 10:32:27 GMT"]},
{"Key":"Server",
"Value":["Apache"]},
{"Key":"Strict-Transport-Security",
"Value":["max-age=63072000;includeSubdomains;"]},
{"Key":"X-Frame-Optio ...
...
.....

@PiemP
Copy link
Contributor

PiemP commented Feb 4, 2025

ciao @Eduardo1977, ho aggiunto una correzione al codice nel commit 70e06fa per evitare la NullReferenceException.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants