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

Sjekk at respons i hentToken har statuskode 2xx #1383

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

dijjal
Copy link
Contributor

@dijjal dijjal commented Sep 5, 2024

Ser fp-oversikt sporadisk feiler på deserialisering av body som ser ut til å komme fra nginx fra plattformen (stopper på første token som er "upstream"). Har ikke funnet hva responskoden er i disse tilfellene, men tenker det ikke er i 200-serien. Legger på en sjekk for å gjøre disse tilfellene mindre forvirrende i loggene.

@dijjal dijjal requested review from mrsladek and jolarsen September 5, 2024 11:31
@dijjal dijjal requested a review from a team as a code owner September 5, 2024 11:31
Copy link
Collaborator

@jolarsen jolarsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kan jo ta med response.body.isEmpty()
Azure/Entra skal returnere innhold dersom 400 , så en sjekk på 2nn er riktig.

@jolarsen
Copy link
Collaborator

jolarsen commented Sep 5, 2024

Hvilken issuer snakker vi om siden vi er i oversikt? TokenX eller Azure? Begge sier at de gir en 400 med response/body iht RFC 8693, Section 2.2.2:

@jolarsen
Copy link
Collaborator

jolarsen commented Sep 5, 2024

Den feiler på deserialisering siden response.body() == "upstream"

@mrsladek
Copy link
Collaborator

mrsladek commented Sep 6, 2024

Jeg tror at Entras response body er i en rar format - men informasjonen der er veldig tydelig, så mulig bare logge response bodyen til secure logg også?

@dijjal
Copy link
Contributor Author

dijjal commented Sep 11, 2024

De aktuelle tilfellene er kall til tokendings. Jeg ser i grafana at vi får 503 fra nginx. Det holder kanskje for vårt analyseformål å bruke det vi får der.

Ang secure logs så tenker vi vel kanskje ikke ha det i bibliotek ettersom ting havner i ordinære logger om secureLogs-config ikke er definert?

@dijjal dijjal force-pushed the sjekk-tokenklient-respons branch from 538b32f to 3350547 Compare September 11, 2024 09:30
@dijjal dijjal merged commit 8f5def5 into master Sep 11, 2024
3 checks passed
@dijjal dijjal deleted the sjekk-tokenklient-respons branch September 11, 2024 09:51
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 this pull request may close these issues.

3 participants