Filter SO dialog search using fnr/dnr

[knuhau]

Issue #322

[arealmaas]

Om min forståelse er riktig her, så vil det å legge ved fødselsnummer "impersonate" en annen identitet(?)

Kunne det vært en ide å navngitt parameteren på en annen måte? Om det ikke er kun en ny måte å filtrere resultater på, men noe som medfører en god del mer logikk. Som f.eks at claims blir rensket. Slik jeg forsto det kan det bli ganske destruktivt om vi ved en feil har med flere claims?

Om vi navngir parameteren impersonateAsSocialSecurityNr eller assumeEndUserWithSocialSecurityNumber / assumeEndUserIdentityWithSocialSecurityNumber så er det kanskje mer klart hva vi faktisk gjør?

Om vi navngir parameteren noe sånt så kan vi lage egne metoder som reflekterer det vi prøver å gjøre også istedenfor å legge ved pID-en som en optional parameter. Det ville gitt mer kontekst per metode iallfall.

Full disclaimer om at jeg kanskje ikke har skjønt det helt 😄

@elsand

[knuhau]

Om min forståelse er riktig her, så vil det å legge ved fødselsnummer "impersonate" en annen identitet(?)

Kunne det vært en ide å navngitt parameteren på en annen måte? Om det ikke er kun en ny måte å filtrere resultater på, men noe som medfører en god del mer logikk. Som f.eks at claims blir rensket. Slik jeg forsto det kan det bli ganske destruktivt om vi ved en feil har med flere claims?

Om vi navngir parameteren impersonateAsSocialSecurityNr eller assumeEndUserWithSocialSecurityNumber / assumeEndUserIdentityWithSocialSecurityNumber så er det kanskje mer klart hva vi faktisk gjør?

Om vi navngir parameteren noe sånt så kan vi lage egne metoder som reflekterer det vi prøver å gjøre også istedenfor å legge ved pID-en som en optional parameter. Det ville gitt mer kontekst per metode iallfall.

Full disclaimer om at jeg kanskje ikke har skjønt det helt 😄

@elsand

Jeg syns Are har gode poenger her, slik det er nå blir det potensielt litt uklart at vi henter ut ressurser "på vegne" av noen andre. Vi kan jo evt. lage en ny metode på IAltinnAuthorization, feks. GetAuthorizedResourcesForSearchForImpersonatedPidfor å tydeliggjøre at vi faktisk henter ut autentiserte ressurser for en helt annen enn den brukeren/tjenesteeieren som foretar spørringen.

Tar høyde for mulige misforståelser her fra min side, dette er fremdeles nytt. Hva tenker du @elsand ?

[elsand]

Enig i at det hadde vært fint om det var tydeligere hva parameteret innebar. Men det er ikke en impersonering, autorisasjonen til tjenesteeieren ligger fremdeles til grunn - det vil si at det kun vil være dialoger som det konkrete tjenesteeiersystemet fra før har tilgang til å aksessere som ligger til grunn for den ytterligere beskrankningen som det å oppgi en sluttbrukers id innebærer. Så tjenesteeieren vil ikke kunne bruke denne mekanismen for å hente ut noe de ikke fra før hadde tilgang til.

Ser også nå (er ikke spesifisert i issuen) at vi må ta høyde for at denne ID-en kan være noe annet enn en ett norsk fnr/dnr - det vil f.eks. kunne være et sluttbrukersystem som vil ha en annen type ID

Så parameteret kan kanskje hete ala filterResourcesAccessibleForEnduserId og bør ha formen som spesifisert i #220, i første omgang bare urn:altinn:person:identifier-no::12345678901 (som inntil #220 implementeres mappes til et pid-claim)

[knuhau]

Enig i at det hadde vært fint om det var tydeligere hva parameteret innebar. Men det er ikke en impersonering, autorisasjonen til tjenesteeieren ligger fremdeles til grunn - det vil si at det kun vil være dialoger som det konkrete tjenesteeiersystemet fra før har tilgang til å aksessere som ligger til grunn for den ytterligere beskrankningen som det å oppgi en sluttbrukers id innebærer. Så tjenesteeieren vil ikke kunne bruke denne mekanismen for å hente ut noe de ikke fra før hadde tilgang til.

Ser også nå (er ikke spesifisert i issuen) at vi må ta høyde for at denne ID-en kan være noe annet enn en ett norsk fnr/dnr - det vil f.eks. kunne være et sluttbrukersystem som vil ha en annen type ID

Så parameteret kan kanskje hete ala filterResourcesAccessibleForEnduserId og bør ha formen som spesifisert i #220, i første omgang bare urn:altinn:person:identifier-no::12345678901 (som inntil #220 implementeres mappes til et pid-claim)

Basert på det du sier her og hvordan navngivningen er på de eksisterende parametrene på Apiet tenker jeg at EndUserId egentlig er dekkende, feks. ved siden av Party og ServiceResource. Siden det kun er en avgrensing/innsnevring av spørringen (og ikke en evt. fundamental endring av hva som returneres som vi trodde først) tenker jeg at EndUserId er tilstrekkelig. (Også siden det må være litt generisk for å dekke evt. andre fremtidige Id'er fra andre systemer).

[arealmaas]

Da gir det mening ja!

EndUserId er nok dekkende da ja. men har en følelse av at resourcesAccessibleForEnduserId angir hvordan vi bruker enduserId, der de andre filtrene er litt selvforklarende 🤔

[elsand]

Ser bra ut dette. Gjør valideringen hakket mer generisk så er vi good to go. Dette får vi ikke testa uansett før Altinn/altinn-authorization#639 er merget. Logisk å se på #220 etter denne er merget.

[elsand]

🚀

[sonarqubecloud]

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud