Fiks feil i maskinporten config som gjør at authenticate ikke kan kjøres i endepunkter #185
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files |
| @@ -14,7 +14,7 @@ enum class Env { | |||
| fun isLocal() = current() == LOCAL | |||
|
|
|||
| fun isMaskinportenDisabled(): Boolean { | |||
There was a problem hiding this comment.
Umiddelbart synes jeg det var litt rart at denne logikken ligger på Env-klassen og at man her bare leser en miljøvariabel direkte i stedet for å gå via application.conf. Er det en grunn til det? Kunne denne logikken heller ligget sammen med resten av maskinporten config-logikken og at verdien av maskinporten.skip kan overstyres av en miljøvariabel?
| jwksUri = ${?MASKINPORTEN_JWKS_URI} | ||
| issuer = ${?MASKINPORTEN_ISSUER} | ||
| scopes = ${?MASKINPORTEN_SCOPES} | ||
| skip = true |
There was a problem hiding this comment.
Ser sånn sett poenget med å fjerne should, men fort gjort å tenke at dette betyr SKIP og ikke har noe med at man skal "hoppe over" autentisering.
Kunne det vært en idé å ha enabled eller disabled i stedet for å unngå noe forvirring?
There was a problem hiding this comment.
er i grunnen enig i det Henrik sier. Ble forvirringer på akkurat dette senest på fredag
| withClaim("scope", config.requiredScopes) | ||
| val shouldDisableMaskinporten = shouldDisableMaskinporten(config) | ||
|
|
||
| skipWhen { shouldDisableMaskinporten } |
There was a problem hiding this comment.
Regner med det er testet, men skjønte ikke helt at man trenger denne når man i tillegg har logikken på linje 22?
There was a problem hiding this comment.
Det ser litt snål ut, men cluet er egentlig bare at vi ønsker å unngå å lese fra config i de tilfellene shouldDisableMaskinporten er true.
Dette er f. eks i dev, hvor vi ikke har satt miljøvariabler, så config.property() vil kræsje om man kaller. Her forsikrer vi oss altså bare om at hvis man først skal skippe, skal man heller ikke lese inn configen
mfw
|
anderssonw
changed the title
Fikser en feil som gjorde at
Authenticationplugin ikke ble installert, som førte til at man ikke kunne kjøre applikasjonen med endepunkter som hadde enauthenticateover seg