-
Notifications
You must be signed in to change notification settings - Fork 2
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
TFP-5952 AntiABAC fase 1, regler og struktur #1408
base: master
Are you sure you want to change the base?
Conversation
import no.nav.vedtak.sikkerhet.kontekst.AnsattGruppe; | ||
|
||
public record PopulasjonInternRequest(UUID ansattOid, | ||
Set<AnsattGruppe> kreverGrupper, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hva er det fptilgang skal bruke kreverGrupper til?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sjekke om oid er med i grupper. Tilgang har Entra-grensesnitt
Fiirst cut er at naiserator bare inneholder veileder/saksbehandler/drift - ikke de øvrige.
if (harGruppe(beskyttetRessursAttributter, AnsattGruppe.DRIFT)) { | ||
return Tilgangsvurdering.godkjenn(); | ||
} else { | ||
return Tilgangsvurdering.godkjenn(AnsattGruppe.DRIFT); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Er det ikke feil å godkjenne hvis bruker ikke har gruppe? Samme gjelder oppgavestyrer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jeg kunne lagt inn en FORTSETT(gruppe) men beholdt GODKJENN(gruppe) i betydning krever gruppe.
Se i PepImpl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Synes "godkjenn" her blir litt feil ord å bruke. For her er det ikke godkjent... men skal vurderes på et senere tidspunkt om er godkjent eller ei. fortsett(AnsattGruppe.DRIFT) eller vurderSenere(AnsattGruppe.DRIFT)? Føler det er mer beskrivende og skaper mindre forvirring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vi får tenke litt på det å heller ta en reformatering senere. Tenker det er greit for nå.
Jeg synes det er litt vanskelig å se hvorfor det er så mange objekter som lagrer data her. Så bruker vi BeskyttetRessursAttributter og AppRessursData i vurderingen. Bare litt forvirret over hva som brukes til hva |
if (!fagtilgang.fikkTilgang()) { | ||
return fagtilgang; | ||
} | ||
kreverGrupper.addAll(fagtilgang.kreverGrupper()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kanskje endre navn til noe mer beskrivende? For dette er grupper som må valideres/sjekkes i populasjonstilgangen siden lokal vurdering ikke fant disse på tokenet.
Eksempler:
grupperSomMåValideresIPopulasjonstilgang
grupperForPopulasjonstilgang
grupperSomMåSjekkesIPopulasjonstilgang
populasjonGrupper
Fase 1
Videre arbeid i fase 1 - implementere i fptilgang og fikse tilgangsklient i denne PR så den kaller.
Deretter rulle ut i 1 app, logge, forbedre, rulle ut alle apps. Ny felles uten abac - rulle ut.
Tilbake: Flytte xacml-koden dit og la k9tilbake kalle som før. Enten med lokalVurdering eller extension av PepImpl.
Kommenter gjerne på struktur, innhold og mangler.